feat(errors): consolidate multiple sarif reports into a single report

Signed-off-by: royalpinto007 <[email protected]> fix(errors): buffer size Signed-off-by: royalpinto007 <[email protected]> fix(errors): diagnostics array Signed-off-by: royalpinto007 <[email protected]> fix(errors): repeated sarif blocks in errors.d and sarif.d Signed-off-by: royalpinto007 <[email protected]> fix(errors): pre-commit checks Signed-off-by: royalpinto007 <[email protected]> fix(errors): circleci build Signed-off-by: royalpinto007 <[email protected]> fix(errors): Ddoc comments and docs Signed-off-by: royalpinto007 <[email protected]> fix(errors): plugSink function by adding early return for empty diagnostics Signed-off-by: royalpinto007 <[email protected]> fix(errors): remove unnecessary change Signed-off-by: royalpinto007 <[email protected]> fix(errors): global array initialisation Signed-off-by: royalpinto007 <[email protected]> fix(errors): separate formatting message Signed-off-by: royalpinto007 <[email protected]> fix(errors): dDoc comments Signed-off-by: royalpinto007 <[email protected]>
dlang · Nov 11, 2024 · dc5f8db · dc5f8db
1 parent b70e660
commit dc5f8db
Show file tree

Hide file tree

Showing 7 changed files with 190 additions and 85 deletions.
diff --git a/compiler/src/dmd/errors.d b/compiler/src/dmd/errors.d
@@ -37,6 +37,19 @@ enum ErrorKind
     message,
 }
 
+/********************************
+ * Represents a diagnostic message generated during compilation, such as errors,
+ * warnings, or other messages.
+ */
+struct Diagnostic
+{
+    SourceLoc loc; // The location in the source code where the diagnostic was generated (includes file, line, and column).
+    string message; // The text of the diagnostic message, describing the issue.
+    ErrorKind kind; // The type of diagnostic, indicating whether it is an error, warning, deprecation, etc.
+}
+
+__gshared Diagnostic[] diagnostics = [];
+
 /***************************
  * Error message sink for D compiler.
  */
@@ -101,6 +114,18 @@ class ErrorSinkCompiler : ErrorSink
         verrorReport(loc, format, ap, ErrorKind.message);
         va_end(ap);
     }
+
+    void plugSink()
+    {
+        // Exit if there are no collected diagnostics
+        if (!diagnostics.length) return;
+
+        // Generate the SARIF report with the current diagnostics
+        generateSarifReport(false);
+
+        // Clear diagnostics after generating the report
+        diagnostics.length = 0;
+    }
 }
 
 
@@ -467,6 +492,7 @@ private extern(C++) void verrorReport(const Loc loc, const(char)* format, va_lis
 private extern(C++) void verrorReport(const SourceLoc loc, const(char)* format, va_list ap, ErrorKind kind, const(char)* p1 = null, const(char)* p2 = null)
 {
     auto info = ErrorInfo(loc, kind, p1, p2);
+
     final switch (info.kind)
     {
     case ErrorKind.error:
@@ -476,7 +502,7 @@ private extern(C++) void verrorReport(const SourceLoc loc, const(char)* format,
             info.headerColor = Classification.error;
             if (global.params.v.messageStyle == MessageStyle.sarif)
             {
-                generateSarifReport(loc, format, ap, info.kind, false);
+                addSarifDiagnostic(loc, format, ap, kind);
                 return;
             }
             verrorPrint(format, ap, info);
@@ -510,7 +536,7 @@ private extern(C++) void verrorReport(const SourceLoc loc, const(char)* format,
                     info.headerColor = Classification.deprecation;
                     if (global.params.v.messageStyle == MessageStyle.sarif)
                     {
-                        generateSarifReport(loc, format, ap, info.kind, false);
+                        addSarifDiagnostic(loc, format, ap, kind);
                         return;
                     }
                     verrorPrint(format, ap, info);
@@ -531,7 +557,7 @@ private extern(C++) void verrorReport(const SourceLoc loc, const(char)* format,
                 info.headerColor = Classification.warning;
                 if (global.params.v.messageStyle == MessageStyle.sarif)
                 {
-                    generateSarifReport(loc, format, ap, info.kind, false);
+                    addSarifDiagnostic(loc, format, ap, kind);
                     return;
                 }
                 verrorPrint(format, ap, info);
@@ -551,7 +577,7 @@ private extern(C++) void verrorReport(const SourceLoc loc, const(char)* format,
             info.headerColor = Classification.tip;
             if (global.params.v.messageStyle == MessageStyle.sarif)
             {
-                generateSarifReport(loc, format, ap, info.kind, false);
+                addSarifDiagnostic(loc, format, ap, kind);
                 return;
             }
             verrorPrint(format, ap, info);
@@ -571,7 +597,7 @@ private extern(C++) void verrorReport(const SourceLoc loc, const(char)* format,
         fflush(stdout);     // ensure it gets written out in case of compiler aborts
         if (global.params.v.messageStyle == MessageStyle.sarif)
         {
-            generateSarifReport(loc, format, ap, info.kind, false);
+            addSarifDiagnostic(loc, format, ap, kind);
             return;
         }
         return;

diff --git a/compiler/src/dmd/frontend.d b/compiler/src/dmd/frontend.d
@@ -180,6 +180,7 @@ void deinitializeDMD()
     import dmd.mtype : Type;
     import dmd.objc : Objc;
     import dmd.target : target;
+    import dmd.errors : diagnostics;
 
     diagnosticHandler = null;
     fatalErrorHandler = null;
@@ -194,6 +195,8 @@ void deinitializeDMD()
     Objc.deinitialize();
     Dsymbol.deinitialize();
     EscapeState.reset();
+
+    diagnostics.length = 0;
 }
 
 /**

diff --git a/compiler/src/dmd/main.d b/compiler/src/dmd/main.d
@@ -177,9 +177,7 @@ private int tryMain(size_t argc, const(char)** argv, ref Param params)
 
         if (global.errors == 0 && global.params.v.messageStyle == MessageStyle.sarif)
         {
-            SourceLoc defaultLoc = SourceLoc(null, 0u, 0u);
-            va_list ap;
-            generateSarifReport(defaultLoc, "", ap, ErrorKind.message, true);
+            generateSarifReport(true);
         }
     }
 

diff --git a/compiler/src/dmd/sarif.d b/compiler/src/dmd/sarif.d
@@ -91,6 +91,29 @@ struct SarifResult {
     }
 }
 
+/**
+Adds a SARIF diagnostic entry to the diagnostics list.
+
+Formats a diagnostic message and appends it to the global diagnostics array, allowing errors, warnings, or other diagnostics to be captured in SARIF format.
+
+Params:
+  loc = The location in the source code where the diagnostic was generated (includes file, line, and column).
+  format = The printf-style format string for the diagnostic message.
+  ap = The variadic argument list containing values to format into the diagnostic message.
+  kind = The type of diagnostic, indicating whether it is an error, warning, deprecation, etc.
+*/
+void addSarifDiagnostic(const SourceLoc loc, const(char)* format, va_list ap, ErrorKind kind) nothrow
+{
+    char[1024] buffer;
+    int written = vsnprintf(buffer.ptr, buffer.length, format, ap);
+
+    // Handle any truncation
+    string formattedMessage = cast(string) buffer[0 .. (written < 0 || written > buffer.length ? buffer.length : written)].dup;
+
+    // Add the Diagnostic to the global diagnostics array
+    diagnostics ~= Diagnostic(loc, formattedMessage, kind);
+}
+
 /// Represents a SARIF report containing tool information, invocation, and results.
 struct SarifReport {
     ToolInformation tool;  /// Information about the analysis tool.
@@ -154,26 +177,41 @@ string formatErrorMessage(const(char)* format, va_list ap) nothrow {
     return buffer[0 .. buffer.length].dup;
 }
 
+/**
+Converts an `ErrorKind` value to a string representation.
+
+Params:
+  kind = The `ErrorKind` value to convert (e.g., error, warning, deprecation).
+
+Returns:
+  A string representing the `ErrorKind` value, such as "Error" or "Warning".
+*/
+string errorKindToString(ErrorKind kind) nothrow
+{
+    final switch (kind) {
+        case ErrorKind.error: return "Error";
+        case ErrorKind.warning: return "Warning";
+        case ErrorKind.deprecation: return "Deprecation";
+        case ErrorKind.tip: return "Tip";
+        case ErrorKind.message: return "Message";
+    }
+}
+
 /**
 Generates a SARIF (Static Analysis Results Interchange Format) report and prints it to `stdout`.
 
-This function builds a JSON-formatted SARIF report, including information about the tool,
-invocation status, error message, severity level, and the location of the issue in the source code.
+This function constructs a JSON-formatted SARIF report that includes information about the tool used (such as compiler version and URI), the invocation status (indicating whether the execution was successful), and a detailed array of diagnostics (results) when `executionSuccessful` is set to `false`. Each diagnostic entry in the results array contains the rule identifier (`ruleId`), a text message describing the issue (`message`), the severity level (`level`), and the location of the issue in the source code, including the file path, line number, and column number. The SARIF report adheres to the SARIF 2.1.0 standard.
 
 Params:
-  loc = The source location where the error occurred (file, line, and column).
-  format = A format string for constructing the error message.
-  ap = A variable argument list used with the format string.
-  kind = The kind of error (error, warning, deprecation, note, or message).
-  executionSuccessful = `true` for an empty `results` array; `false` for detailed errors.
+   executionSuccessful = `true` for an empty `results` array; `false` for detailed errors.
 
 Throws:
   This function is marked as `nothrow` and does not throw exceptions.
 
 See_Also:
   $(LINK2 https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0.json, SARIF 2.1.0 schema)
 */
-void generateSarifReport(const ref SourceLoc loc, const(char)* format, va_list ap, ErrorKind kind, bool executionSuccessful) nothrow
+void generateSarifReport(bool executionSuccessful) nothrow
 {
     // Create an OutBuffer to store the SARIF report
     OutBuffer ob;
@@ -194,7 +232,6 @@ void generateSarifReport(const ref SourceLoc loc, const(char)* format, va_list a
     string cleanedVersion = toolVersion[0 .. length];
 
     // Build SARIF report
-    ob.level = 0;
     ob.writestringln("{");
     ob.level = 1;
 
@@ -235,65 +272,30 @@ void generateSarifReport(const ref SourceLoc loc, const(char)* format, va_list a
     ob.level -= 1;
     ob.writestringln("}],");
 
-    // Empty results array for successful execution
-    if (executionSuccessful)
-    {
-        ob.writestringln(`"results": []`);
-    }
-    // Error information if execution was unsuccessful
-    else
-    {
-        // Format the error message
-        string formattedMessage = formatErrorMessage(format, ap);
-
-        // Map ErrorKind to SARIF levels
-        const(char)* level;
-        string ruleId;
-        final switch (kind) {
-            case ErrorKind.error:
-                level = "error";
-                ruleId = "DMD-ERROR";
-                break;
-            case ErrorKind.warning:
-                level = "warning";
-                ruleId = "DMD-WARNING";
-                break;
-            case ErrorKind.deprecation:
-                level = "deprecation";
-                ruleId = "DMD-DEPRECATION";
-                break;
-            case ErrorKind.tip:
-                level = "note";
-                ruleId = "DMD-NOTE";
-                break;
-            case ErrorKind.message:
-                level = "none";
-                ruleId = "DMD-MESSAGE";
-                break;
-        }
+    // Results Array
+    ob.writestringln(`"results": [`);
+    ob.level += 1;
 
-        // Results Array for errors
-        ob.writestringln(`"results": [{`);
+    foreach (idx, diag; diagnostics) {
+        ob.writestringln("{");
         ob.level += 1;
 
         // Rule ID
-        ob.writestring(`"ruleId": "`);
-        ob.writestring(ruleId);
-        ob.writestringln(`",`);
+        ob.writestring(`"ruleId": "DMD-` ~ errorKindToString(diag.kind) ~ `",`);
+        ob.writestringln("");
 
         // Message Information
         ob.writestringln(`"message": {`);
         ob.level += 1;
         ob.writestring(`"text": "`);
-        ob.writestring(formattedMessage.ptr);
+        ob.writestring(diag.message);
         ob.writestringln(`"`);
         ob.level -= 1;
         ob.writestringln("},");
 
         // Error Severity Level
-        ob.writestring(`"level": "`);
-        ob.writestring(level);
-        ob.writestringln(`",`);
+        ob.writestring(`"level": "` ~ errorKindToString(diag.kind) ~ `",`);
+        ob.writestringln("");
 
         // Location Information
         ob.writestringln(`"locations": [{`);
@@ -305,7 +307,7 @@ void generateSarifReport(const ref SourceLoc loc, const(char)* format, va_list a
         ob.writestringln(`"artifactLocation": {`);
         ob.level += 1;
         ob.writestring(`"uri": "`);
-        ob.writestring(loc.filename);
+        ob.writestring(diag.loc.filename);
         ob.writestringln(`"`);
         ob.level -= 1;
         ob.writestringln("},");
@@ -314,10 +316,10 @@ void generateSarifReport(const ref SourceLoc loc, const(char)* format, va_list a
         ob.writestringln(`"region": {`);
         ob.level += 1;
         ob.writestring(`"startLine": `);
-        ob.printf(`%d,`, loc.linnum);
+        ob.printf(`%d,`, diag.loc.linnum);
         ob.writestringln("");
         ob.writestring(`"startColumn": `);
-        ob.printf(`%d`, loc.charnum);
+        ob.printf(`%d`, diag.loc.charnum);
         ob.writestringln("");
         ob.level -= 1;
         ob.writestringln("}");
@@ -327,14 +329,22 @@ void generateSarifReport(const ref SourceLoc loc, const(char)* format, va_list a
         ob.writestringln("}");
         ob.level -= 1;
         ob.writestringln("}]");
+
+        // Closing brace for each diagnostic item
         ob.level -= 1;
-        ob.writestringln("}]");
+        if (idx < diagnostics.length - 1) {
+            ob.writestringln("},");
+        } else {
+            ob.writestringln("}");
+        }
     }
 
     // Close the run and SARIF JSON
     ob.level -= 1;
+    ob.writestringln("]");
+    ob.level -= 1;
     ob.writestringln("}]");
-    ob.level = 0;
+    ob.level -= 1;
     ob.writestringln("}");
 
     // Extract the final null-terminated string and print it to stdout

diff --git a/compiler/test/compilable/sarif_success_test.d b/compiler/test/compilable/sarif_success_test.d
@@ -16,7 +16,8 @@ TEST_OUTPUT:
 		"invocations": [{
 			"executionSuccessful": true
 		}],
-		"results": []
+		"results": [
+		]
 	}]
 }
 ---

diff --git a/compiler/test/fail_compilation/sarif_test.d b/compiler/test/fail_compilation/sarif_test.d
@@ -16,24 +16,26 @@ TEST_OUTPUT:
 		"invocations": [{
 			"executionSuccessful": false
 		}],
-		"results": [{
-			"ruleId": "DMD-ERROR",
-			"message": {
-				"text": "undefined identifier `x`"
-			},
-			"level": "error",
-			"locations": [{
-				"physicalLocation": {
-					"artifactLocation": {
-						"uri": "fail_compilation/sarif_test.d"
-					},
-					"region": {
-						"startLine": 43,
-						"startColumn": 5
+		"results": [
+			{
+				"ruleId": "DMD-Error",
+				"message": {
+					"text": "undefined identifier `x`"
+				},
+				"level": "Error",
+				"locations": [{
+					"physicalLocation": {
+						"artifactLocation": {
+							"uri": "fail_compilation/sarif_test.d"
+						},
+						"region": {
+							"startLine": 45,
+							"startColumn": 5
+						}
 					}
-				}
-			}]
-		}]
+				}]
+			}
+		]
 	}]
 }
 ---