fix: parse error serialisation (#420)

Convert parse errors into strings manually to avoid JSON serialisation failure due to circular references. Fixes #406
nvuillam · Aug 29, 2024 · fef3dc2 · fef3dc2
1 parent 6ec453b
commit fef3dc2
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 9 deletions.
diff --git a/groovy/src/main/com/nvuillam/CodeNarcServer.groovy b/groovy/src/main/com/nvuillam/CodeNarcServer.groovy
@@ -194,7 +194,7 @@ class CodeNarcServer {
                     WRITER.writeValue(out, response)
                 }
             } catch (Exception e) {
-                LOGGER.error('Write response {}', e)
+                LOGGER.error('Write response', e)
             } finally {
                 if (requestKey) {
                     threads.remove(requestKey)

diff --git a/groovy/src/main/com/nvuillam/Request.groovy b/groovy/src/main/com/nvuillam/Request.groovy
@@ -145,8 +145,8 @@ class Request {
      * @param fileList the list of files to parse
      * @return the map of files to errors
      */
-    private Map<String, List<Error>> parseFiles(List<String> fileList) {
-        Map<String, List<Error>> parseErrors = [:]
+    private Map<String, List<String>> parseFiles(List<String> fileList) {
+        Map<String, List<String>> parseErrors = [:]
         LOGGER.debug('parseFiles: parse={}, fileList={}', parse, fileList)
         if (parse) {
             fileList.each { file ->
@@ -210,8 +210,21 @@ class Request {
         return files
     }
 
-    // Try to parse file to get compilation errors
-    private List<Error> parseFile(File file) {
+    // Try to parse file to get compilation errors as strings.
+    private List<String> parseFile(File file) {
+        List<String> errors = []
+        for (err in parseFileErrors(file)) {
+            StringWriter out = new StringWriter();
+            PrintWriter writer = new PrintWriter(out);
+            err.write(writer)
+            errors << out.toString()
+        }
+
+        return errors
+    }
+
+    // Try to parse file to get compilation errors.
+    private List<Error> parseFileErrors(File file) {
         try {
             // We don't use GroovyShell.parse as it calls InvokerHelper.createScript
             // which fails for files which contain a class which only have non-zero

diff --git a/groovy/src/main/com/nvuillam/Response.groovy b/groovy/src/main/com/nvuillam/Response.groovy
@@ -23,7 +23,7 @@ class Response {
     String errorMessage
     String errorDtl
     String exceptionType
-    Map<String, List<Error>> parseErrors
+    Map<String, List<String>> parseErrors
 
     // Lint result.
     @JsonRawValue

diff --git a/lib/java/CodeNarcServer.jar b/lib/java/CodeNarcServer.jar
diff --git a/test/lint-api.test.js b/test/lint-api.test.js
@@ -316,7 +316,7 @@ describe("Lint with API", () => {
         assert(linter.status === 1, `Linter status is 1 (${linter.status} returned)`);
         assert(linter.outputString.includes("warning"), "Output string contains warning");
         assert(Object.keys(linter.lintResult.files).length === 2, "Files array contains 2 files");
-        assert(linter.lintResult.summary.totalFoundErrorNumber === 4, "Error found");
+        assert(linter.lintResult.summary.totalFoundErrorNumber === 5, "Error found");
         assert(linter.lintResult.summary.totalFoundWarningNumber === 6, "Warnings found");
         assert(linter.lintResult.summary.totalFoundInfoNumber === 65, "Infos found");
         checkCodeNarcCallsCounter(1);
@@ -354,7 +354,7 @@ describe("Lint with API", () => {
         assert(linter.status === 1, `Linter status is 1 (${linter.status} returned)`);
         assert(linter.outputString.includes("warning"), "Output string contains warning");
         assert(Object.keys(linter.lintResult.files).length === 2, "Files array contains 2 files");
-        assert(linter.lintResult.summary.totalFoundErrorNumber === 4, "Error found");
+        assert(linter.lintResult.summary.totalFoundErrorNumber === 5, "Error found");
         assert(linter.lintResult.summary.totalFoundWarningNumber === 6, "Warnings found");
         assert(linter.lintResult.summary.totalFoundInfoNumber === 65, "Infos found");
         checkCodeNarcCallsCounter(1);
@@ -372,7 +372,7 @@ describe("Lint with API", () => {
         assert(linter.status === 1, `Linter status is 1 (${linter.status} returned)`);
         assert(linter.outputString.includes("warning"), "Output string contains warning");
         assert(Object.keys(linter.lintResult.files).length === 12, `Expected 2 files got ${Object.keys(linter.lintResult.files).length}`);
-        assert(linter.lintResult.summary.totalFoundErrorNumber === 12, `Expected 12 errors to ${linter.lintResult.summary.totalFoundErrorNumber}`);
+        assert(linter.lintResult.summary.totalFoundErrorNumber === 19, `Expected 19 errors to ${linter.lintResult.summary.totalFoundErrorNumber}`);
         assert(linter.lintResult.summary.totalFoundWarningNumber === 333, `Expected 333 warnings to ${linter.lintResult.summary.totalFoundWarningNumber}`);
         assert(linter.lintResult.summary.totalFoundInfoNumber === 1649, `Expected 1649 infos to ${linter.lintResult.summary.totalFoundInfoNumber}`);
         checkCodeNarcCallsCounter(1);