Include "clang" diagnostics from SourceKit-LSP (#1062)

award999 · AppAppWorks · web-flow · commit 6efb5cedf359 · 2024-09-23T09:18:02.000-04:00
* Include "clang" diagnostics from SourceKit-LSP * Include "clang" in the list of diagnostics sources * Fix regex to not include "[-<code>]" that compiler outputs for C/C++ diagnostics * Clean up the "(fix available)" SourceKit includes in clang messages so that merging works proper * Add new tests to watch for clang diagnostics Issue: #1028 * Add some tests for SourceKit-LSP * Make sure we're getting diagnostics * Realized was an issue where "instanceof" does not work when the extension under test is using source under "dist/" but the tests use source under "out" so if test instantiated an object, instanceof may not work in the extension code * Fix pretest hook for clean repo * fixed missing diagnostics from clangd - fixed #1028 - diagnostic sources other than "swiftc" all regarded as SourceKit * Prevent new launch configurations when running integration tests --------- Co-authored-by: Kai Lau <kai@appapp.works>
diff --git a/assets/test/.vscode/settings.json b/assets/test/.vscode/settings.json
@@ -1,5 +1,6 @@
 {
     "swift.disableAutoResolve": true,
+    "swift.autoGenerateLaunchConfigurations": false,
     "swift.additionalTestArguments": [
         "-Xswiftc",
         "-DTEST_ARGUMENT_SET_VIA_TEST_BUILD_ARGUMENTS_SETTING"
diff --git a/assets/test/diagnosticsC/Package.swift b/assets/test/diagnosticsC/Package.swift
@@ -0,0 +1,12 @@
+// swift-tools-version:5.6
+import PackageDescription
+
+let package = Package(
+    name: "MyPoint",
+    products: [
+        .library(name: "MyPoint", targets: ["MyPoint"]),
+    ],
+    targets: [
+        .target(name: "MyPoint"),
+    ]
+)
diff --git a/assets/test/diagnosticsC/Sources/MyPoint/MyPoint.c b/assets/test/diagnosticsC/Sources/MyPoint/MyPoint.c
@@ -0,0 +1,9 @@
+#include "MyPoint.h"
+
+int func() {
+    struct MyPoint p;
+    p.x = 1;
+    p.y = bar;
+    p.z = 2
+    return p.x;
+}
diff --git a/assets/test/diagnosticsC/Sources/MyPoint/include/MyPoint.h b/assets/test/diagnosticsC/Sources/MyPoint/include/MyPoint.h
@@ -0,0 +1,4 @@
+struct MyPoint {
+   int x;
+   int y;
+};
diff --git a/assets/test/diagnosticsCpp/Package.swift b/assets/test/diagnosticsCpp/Package.swift
@@ -0,0 +1,12 @@
+// swift-tools-version:5.6
+import PackageDescription
+
+let package = Package(
+    name: "MyPoint",
+    products: [
+        .library(name: "MyPoint", targets: ["MyPoint"]),
+    ],
+    targets: [
+        .target(name: "MyPoint"),
+    ]
+)
diff --git a/assets/test/diagnosticsCpp/Sources/MyPoint/MyPoint.cpp b/assets/test/diagnosticsCpp/Sources/MyPoint/MyPoint.cpp
@@ -0,0 +1,9 @@
+#include "MyPoint.h"
+
+int func() {
+    MyPoint* p = new MyPoint2();
+    p->x = 1;
+    p->y = bar;
+    p.z = 2
+    return p.x;
+}
diff --git a/assets/test/diagnosticsCpp/Sources/MyPoint/include/MyPoint.h b/assets/test/diagnosticsCpp/Sources/MyPoint/include/MyPoint.h
@@ -0,0 +1,5 @@
+class MyPoint {
+   public:
+      int x;
+      int y;
+};
diff --git a/package.json b/package.json
@@ -1275,7 +1275,7 @@
     "watch": "tsc --watch",
     "lint": "eslint ./ --ext ts && tsc --noEmit",
     "format": "prettier --check *.json src test",
-    "pretest": "npm run compile && del-cli ./assets/test/**/.build && tsc -p ./",
+    "pretest": "npm run compile-tests && del-cli ./assets/test/**/.build && tsc -p ./",
     "test": "vscode-test",
     "integration-test": "npm test -- --label integrationTests",
     "unit-test": "npm test -- --label unitTests",
diff --git a/src/DiagnosticsManager.ts b/src/DiagnosticsManager.ts
@@ -26,9 +26,22 @@ interface ParsedDiagnostic {
 
 type DiagnosticsMap = Map<string, vscode.Diagnostic[]>;
 
+type SourcePredicate = (source: string) => boolean;
+
+type DiagnosticPredicate = (diagnostic: vscode.Diagnostic) => boolean;
+
 const isEqual = (d1: vscode.Diagnostic, d2: vscode.Diagnostic) =>
     d1.range.start.isEqual(d2.range.start) && d1.message === d2.message;
 
+const isSource = (diagnostic: vscode.Diagnostic, sourcesPredicate: SourcePredicate) =>
+    sourcesPredicate(diagnostic.source ?? "");
+
+const isSwiftc: DiagnosticPredicate = diagnostic =>
+    isSource(diagnostic, DiagnosticsManager.isSwiftc);
+
+const isSourceKit: DiagnosticPredicate = diagnostic =>
+    isSource(diagnostic, DiagnosticsManager.isSourcekit);
+
 /**
  * Handles the collection and deduplication of diagnostics from
  * various {@link vscode.Diagnostic.source | Diagnostic sources}.
@@ -38,9 +51,9 @@ const isEqual = (d1: vscode.Diagnostic, d2: vscode.Diagnostic) =>
  * thier own diagnostics.
  */
 export class DiagnosticsManager implements vscode.Disposable {
-    // Prior to Swift 6 "sourcekitd" was the source
-    static sourcekit: string[] = ["SourceKit", "sourcekitd"];
-    static swiftc: string[] = ["swiftc"];
+    private static swiftc: string = "swiftc";
+    static isSourcekit: SourcePredicate = source => this.swiftc !== source;
+    static isSwiftc: SourcePredicate = source => this.swiftc === source;
 
     private diagnosticCollection: vscode.DiagnosticCollection =
         vscode.languages.createDiagnosticCollection("swift");
@@ -57,15 +70,15 @@ export class DiagnosticsManager implements vscode.Disposable {
         });
         this.onDidStartTaskDisposible = vscode.tasks.onDidStartTask(event => {
             // Will only try to provide diagnostics for `swift` tasks
-            const execution = event.execution.task.execution;
-            if (!(execution && execution instanceof SwiftExecution)) {
+            const task = event.execution.task;
+            if (task.definition.type !== "swift") {
                 return;
             }
             if (!this.includeSwiftcDiagnostics()) {
                 return;
             }
             // Provide new list of diagnostics
-            const swiftExecution = execution as SwiftExecution;
+            const swiftExecution = task.execution as SwiftExecution;
             const provideDiagnostics: Promise<DiagnosticsMap> =
                 this.parseDiagnostics(swiftExecution);
 
@@ -76,7 +89,7 @@ export class DiagnosticsManager implements vscode.Disposable {
                     map.forEach((diagnostics, uri) =>
                         this.handleDiagnostics(
                             vscode.Uri.file(uri),
-                            DiagnosticsManager.swiftc,
+                            DiagnosticsManager.isSwiftc,
                             diagnostics
                         )
                     );
@@ -91,25 +104,25 @@ export class DiagnosticsManager implements vscode.Disposable {
      * Provide a new list of diagnostics for a given file
      *
      * @param uri {@link vscode.Uri Uri} of the file these diagonstics apply to
-     * @param sources The source of the diagnostics which will apply for cleaning
-     * up diagnostics that have been removed. See {@link swiftc} and {@link sourcekit}
-     * @param newDiagnostics Array of {@link vscode.Diagnostic}. This can be empty to remove old diagnostics for the specified `sources`.
+     * @param sourcePredicate Diagnostics of a source that satisfies the predicate will apply for cleaning
+     * up diagnostics that have been removed. See {@link isSwiftc} and {@link isSourceKit}
+     * @param newDiagnostics Array of {@link vscode.Diagnostic}. This can be empty to remove old diagnostics satisfying `sourcePredicate`.
      */
     handleDiagnostics(
         uri: vscode.Uri,
-        sources: string[],
+        sourcePredicate: SourcePredicate,
         newDiagnostics: vscode.Diagnostic[]
     ): void {
-        const isFromSourceKit = !!DiagnosticsManager.sourcekit.find(s => sources.includes(s));
+        const isFromSourceKit = !sourcePredicate(DiagnosticsManager.swiftc);
         // Is a descrepency between SourceKit-LSP and older versions
         // of Swift as to whether the first letter is capitalized or not,
         // so we'll always display messages capitalized to user and this
         // also will allow comparing messages when merging
-        newDiagnostics = newDiagnostics.map(this.capitalizeMessage);
+        newDiagnostics = newDiagnostics.map(this.capitalizeMessage).map(this.cleanMessage);
         const allDiagnostics = this.allDiagnostics.get(uri.fsPath)?.slice() || [];
         // Remove the old set of diagnostics from this source
         const removedDiagnostics = this.removeDiagnostics(allDiagnostics, d =>
-            this.isSource(d, sources)
+            isSource(d, sourcePredicate)
         );
         // Clean up any "fixed" swiftc diagnostics
         if (isFromSourceKit) {
@@ -119,7 +132,7 @@ export class DiagnosticsManager implements vscode.Disposable {
             );
             this.removeDiagnostics(
                 allDiagnostics,
-                d1 => this.isSwiftc(d1) && !!removedDiagnostics.find(d2 => isEqual(d1, d2))
+                d1 => isSwiftc(d1) && !!removedDiagnostics.find(d2 => isEqual(d1, d2))
             );
         }
         // Append the new diagnostics we just received
@@ -131,25 +144,17 @@ export class DiagnosticsManager implements vscode.Disposable {
 
     private updateDiagnosticsCollection(uri: vscode.Uri): void {
         const diagnostics = this.allDiagnostics.get(uri.fsPath) ?? [];
-        const swiftcDiagnostics = diagnostics.filter(d => this.isSwiftc(d));
-        const sourceKitDiagnostics = diagnostics.filter(d => this.isSourceKit(d));
+        const swiftcDiagnostics = diagnostics.filter(isSwiftc);
+        const sourceKitDiagnostics = diagnostics.filter(isSourceKit);
         const mergedDiagnostics: vscode.Diagnostic[] = [];
         switch (configuration.diagnosticsCollection) {
             case "keepSourceKit":
                 mergedDiagnostics.push(...swiftcDiagnostics);
-                this.mergeDiagnostics(
-                    mergedDiagnostics,
-                    sourceKitDiagnostics,
-                    DiagnosticsManager.sourcekit
-                );
+                this.mergeDiagnostics(mergedDiagnostics, sourceKitDiagnostics, isSourceKit);
                 break;
             case "keepSwiftc":
                 mergedDiagnostics.push(...sourceKitDiagnostics);
-                this.mergeDiagnostics(
-                    mergedDiagnostics,
-                    swiftcDiagnostics,
-                    DiagnosticsManager.swiftc
-                );
+                this.mergeDiagnostics(mergedDiagnostics, swiftcDiagnostics, isSwiftc);
                 break;
             case "onlySourceKit":
                 mergedDiagnostics.push(...sourceKitDiagnostics);
@@ -168,7 +173,7 @@ export class DiagnosticsManager implements vscode.Disposable {
     private mergeDiagnostics(
         mergedDiagnostics: vscode.Diagnostic[],
         newDiagnostics: vscode.Diagnostic[],
-        precedence: string[]
+        precedencePredicate: DiagnosticPredicate
     ): void {
         for (const diagnostic of newDiagnostics) {
             // See if a duplicate diagnostic exists
@@ -178,11 +183,11 @@ export class DiagnosticsManager implements vscode.Disposable {
             }
 
             // Perform de-duplication
-            if (precedence.includes(diagnostic.source || "")) {
+            if (precedencePredicate(diagnostic)) {
                 mergedDiagnostics.push(diagnostic);
                 continue;
             }
-            if (!currentDiagnostic || !precedence.includes(currentDiagnostic.source || "")) {
+            if (!currentDiagnostic || !precedencePredicate(currentDiagnostic)) {
                 mergedDiagnostics.push(diagnostic);
                 continue;
             }
@@ -193,29 +198,17 @@ export class DiagnosticsManager implements vscode.Disposable {
     private removeSwiftcDiagnostics() {
         this.allDiagnostics.forEach((diagnostics, path) => {
             const newDiagnostics = diagnostics.slice();
-            this.removeDiagnostics(newDiagnostics, d => this.isSwiftc(d));
+            this.removeDiagnostics(newDiagnostics, isSwiftc);
             if (diagnostics.length !== newDiagnostics.length) {
                 this.allDiagnostics.set(path, newDiagnostics);
             }
             this.updateDiagnosticsCollection(vscode.Uri.file(path));
         });
     }
 
-    private isSource(diagnostic: vscode.Diagnostic, sources: string[]): boolean {
-        return sources.includes(diagnostic.source || "");
-    }
-
-    private isSwiftc(diagnostic: vscode.Diagnostic): boolean {
-        return this.isSource(diagnostic, DiagnosticsManager.swiftc);
-    }
-
-    private isSourceKit(diagnostic: vscode.Diagnostic): boolean {
-        return this.isSource(diagnostic, DiagnosticsManager.sourcekit);
-    }
-
     private removeDiagnostics(
         diagnostics: vscode.Diagnostic[],
-        matches: (d: vscode.Diagnostic) => boolean
+        matches: DiagnosticPredicate
     ): vscode.Diagnostic[] {
         const removed: vscode.Diagnostic[] = [];
         let i = diagnostics.length;
@@ -323,13 +316,13 @@ export class DiagnosticsManager implements vscode.Disposable {
     private parseDiagnostic(
         line: string
     ): ParsedDiagnostic | vscode.DiagnosticRelatedInformation | undefined {
-        const diagnosticRegex = /^(.*?):(\d+)(?::(\d+))?:\s+(warning|error|note):\s+(.*)$/g;
+        const diagnosticRegex = /^(.*?):(\d+)(?::(\d+))?:\s+(warning|error|note):\s+([^\\[]*)/g;
         const match = diagnosticRegex.exec(line);
         if (!match) {
             return;
         }
         const uri = match[1];
-        const message = this.capitalize(match[5]);
+        const message = this.capitalize(match[5]).trim();
         const range = this.range(match[2], match[3]);
         const severity = this.severity(match[4]);
         if (severity === vscode.DiagnosticSeverity.Information) {
@@ -339,7 +332,7 @@ export class DiagnosticsManager implements vscode.Disposable {
             );
         }
         const diagnostic = new vscode.Diagnostic(range, message, severity);
-        diagnostic.source = DiagnosticsManager.swiftc[0];
+        diagnostic.source = DiagnosticsManager.swiftc;
         return { uri, diagnostic };
     }
 
@@ -377,6 +370,12 @@ export class DiagnosticsManager implements vscode.Disposable {
         return diagnostic;
     };
 
+    private cleanMessage = (diagnostic: vscode.Diagnostic) => {
+        diagnostic = { ...diagnostic };
+        diagnostic.message = diagnostic.message.replace("(fix available)", "").trim();
+        return diagnostic;
+    };
+
     private onDidStartTaskDisposible: vscode.Disposable;
     private onDidChangeConfigurationDisposible: vscode.Disposable;
 }
diff --git a/src/sourcekit-lsp/LanguageClientManager.ts b/src/sourcekit-lsp/LanguageClientManager.ts
@@ -545,15 +545,15 @@ export class LanguageClientManager {
                     const document = uri as vscode.TextDocument;
                     this.workspaceContext.diagnostics.handleDiagnostics(
                         document.uri ?? uri,
-                        DiagnosticsManager.sourcekit,
+                        DiagnosticsManager.isSourcekit,
                         result?.items ?? []
                     );
                     return undefined;
                 },
                 handleDiagnostics: (uri, diagnostics) => {
                     this.workspaceContext.diagnostics.handleDiagnostics(
                         uri,
-                        DiagnosticsManager.sourcekit,
+                        DiagnosticsManager.isSourcekit,
                         diagnostics
                     );
                 },
diff --git a/src/ui/SwiftBuildStatus.ts b/src/ui/SwiftBuildStatus.ts
@@ -52,7 +52,7 @@ export class SwiftBuildStatus implements vscode.Disposable {
 
     private handleTaskStatus(task: vscode.Task): void {
         // Only care about swift tasks
-        if (!(task.execution && task.execution instanceof SwiftExecution)) {
+        if (task.definition.type !== "swift") {
             return;
         }
         // Default to setting if task doesn't overwrite
diff --git a/test/suite/DiagnosticsManager.test.ts b/test/suite/DiagnosticsManager.test.ts
diff --git a/test/suite/commands/runTestMultipleTimes.test.ts b/test/suite/commands/runTestMultipleTimes.test.ts
diff --git a/test/suite/extension.test.ts b/test/suite/extension.test.ts
diff --git a/test/unit-tests/ui/SwiftBuildStatus.test.ts b/test/unit-tests/ui/SwiftBuildStatus.test.ts

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"swift.disableAutoResolve": true,`
	`3`	`+ "swift.autoGenerateLaunchConfigurations": false,`
`3`	`4`	`"swift.additionalTestArguments": [`
`4`	`5`	`"-Xswiftc",`
`5`	`6`	`"-DTEST_ARGUMENT_SET_VIA_TEST_BUILD_ARGUMENTS_SETTING"`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +struct MyPoint {
 +   int x;
 +   int y;
 +};