Improved error parsing when using verilator

CHANGELOG.md

@@ -2,7 +2,17 @@
 
 All notable changes to this project will be documented in this file.
 
-The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
+The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)\
+
+## [Unreleased]
+
+## Added
+- Verilator linter stderr passthrough [#489](https://github.com/mshr-h/vscode-verilog-hdl-support/issues/489)
+- When linting using Verilator, all detected problems are highlighted (By default it's just current file and it's dependencies. Verilator launch config can be adjusted in linting settings)
+
+## Fixed
+- Imroved regex matcher for Verilator output
+- Verilator output blocks are correctly tagged with `[error]` or `[warning]`
 
 ## [1.14.2] - 2024-07-24
 

src/linter/VerilatorLinter.ts

@@ -103,46 +103,156 @@ export default class VerilatorLinter extends BaseLinter {
     this.logger.info('[verilator]   command: ' + command);
     this.logger.info('[verilator]   cwd    : ' + cwd);
 
+
+
     var _: child.ChildProcess = child.exec(
       command,
       { cwd: cwd },
       (_error: Error, _stdout: string, stderr: string) => {
-        let diagnostics: vscode.Diagnostic[] = [];
-        stderr.split(/\r?\n/g).forEach((line, _) => {
-          if (line.search("No such file or directory") >= 0 || line.search("Not a directory") >= 0 || line.search("command not found") >= 0) {
-            this.logger.error(`Could not execute command: ${command}`);
-            return;
+
+        // basically DiagnosticsCollection but with ability to append diag lists
+        let filesDiag = new Map();
+
+        stderr.split(/\r?\n/g).forEach((line, _, stderrLines) => {
+
+
+          // if lineIndex is 0 and it doesn't start with %Error or %Warning,
+          // the whole loop would skip
+          // and it is probably a system error (wrong file name/directory/something)
+          let lastDiagMessageType: string = "Error";
+
+          // parsing previous lines for message type
+          // shouldn't be more than 5 or so
+          for (let lineIndex = _; lineIndex >= 0; lineIndex--)
+          {
+            if (stderrLines[lineIndex].startsWith("%Error"))
+            {
+              lastDiagMessageType = "Error";
+              break;
+            }
+            if (stderrLines[lineIndex].startsWith("%Warning"))
+            {
+              lastDiagMessageType = "Warning";
+              break;
+            }
           }
 
-          if (!line.startsWith('%') || line.indexOf(docUri) <= 0) {
+          // first line would be normal stderr output like "directory name is invalid"
+          // others are verilator sort of "highlighting" the issue, the block with "^~~~~"
+          // this can actually be used for better error/warning highlighting
+
+          // also this might have some false positives
+          // probably something like "stderr passthrough setting" would be a good idea
+          if (!line.startsWith('%')) {
+
+            // allows for persistent 
+            if (lastDiagMessageType === 'Warning') { this.logger.warn(line); }
+              else { this.logger.error(line); }
             return;
           }
 
-          let rex = line.match(
-            /%(\w+)(-[A-Z0-9_]+)?:\s*(\w+:)?(?:[^:]+):\s*(\d+):(?:\s*(\d+):)?\s*(\s*.+)/
+
+          // important match sections are named now:
+          // severity - Error or Warning
+          // errorCode - error code, if there is one, something like PINNOTFOUND
+          // filePath - full path to the file, including it's name and extension
+          // lineNumber - line number
+          // columNumber - columnNumber
+          // verboseError - error elaboration by verilator
+
+          let errorParserRegex = new RegExp(
+            /%(?<severity>\w+)/.source + // matches "%Warning" or "%Error"
+
+            // this matches errorcode with "-" before it, but the "-" doesn't go into ErrorCode match group
+            /(-(?<errorCode>[A-Z0-9]+))?/.source + // matches error code like -PINNOTFOUND
+
+            /: /.source + // ": " before file path or error message
+
+            // this one's a bit of a mess, but apparently one can't cleanly split regex match group between lines
+            // and this is a large group since it matches file path and line and column numbers which may not exist at all
+
+            // note: end of file path is detected using file extension at the end of it
+            // this also allows for spaces in path.
+            // (neiter Linux, nor Windows actually prohibits it, and Verilator handles spaces just fine)
+            // In my testing, didn't lead cause any problems, but it potentially can
+            // extension names are placed so that longest one is first and has highest priority
+
+            /((?<filePath>(\S| )+(?<fileExtension>(\.svh)|(\.sv)|(\.SV)|(\.vh)|(\.vl)|(\.v))):((?<lineNumber>\d+):)?((?<columnNumber>\d+):)? )?/.source +
+
+            // matches error message produced by Verilator
+            /(?<verboseError>.*)/.source
+            , "g"
           );
 
+          let rex = errorParserRegex.exec(line);
+
+          // stderr passthrough
+          // probably better toggled with a parameter
+          if (rex.groups["severity"] === "Error") { this.logger.error(line); }
+            else if (rex.groups["severity"] === "Warning") { this.logger.warn(line); }
+
+            // theoretically, this shoudn't "fire", but just in case
+            else { this.logger.error(line); }
+
+
+
+
+          // vscode problems are tied to files
+          // if there isn't a file name, no point going further
+          if (!rex.groups["filePath"]) {
+            return;
+          }
+
+          // replacing "\\" and "\" with "/" for consistency
+          if (isWindows)
+          {
+            rex.groups["filePath"] = rex.groups["filePath"].replace(/(\\\\)|(\\)/g, "/");
+          }
+
+          // if there isn't a list of errors for this file already, it
+          // needs to be created
+          if (!filesDiag.has(rex.groups["filePath"]))
+          {
+            filesDiag.set(rex.groups["filePath"], []);
+          }
+
+
           if (rex && rex[0].length > 0) {
-            let lineNum = Number(rex[4]) - 1;
-            let colNum = Number(rex[5]) - 1;
-            // Type of warning is in rex[2]
+            let lineNum = Number(rex.groups["lineNumber"]) - 1;
+            let colNum = Number(rex.groups["columnNumber"]) - 1;
+
             colNum = isNaN(colNum) ? 0 : colNum; // for older Verilator versions (< 4.030 ~ish)
 
             if (!isNaN(lineNum)) {
-              diagnostics.push({
-                severity: this.convertToSeverity(rex[1]),
+
+              // appending diagnostic message to an array of messages
+              // tied to a file
+              filesDiag.get(rex.groups["filePath"]).push({
+                severity: this.convertToSeverity(rex.groups["severity"]),
                 range: new vscode.Range(lineNum, colNum, lineNum, Number.MAX_VALUE),
-                message: rex[6],
-                code: 'verilator',
+                message: rex.groups["verboseError"],
+                code: rex.groups["errorCode"],
                 source: 'verilator',
               });
+
             }
             return;
           }
-          this.logger.warn('[verilator] failed to parse error: ' + line);
         });
-        this.logger.info(`[verilator] ${diagnostics.length} errors/warnings returned`);
-        this.diagnosticCollection.set(doc.uri, diagnostics);
+
+        // since error parsing has been redone "from the ground up"
+        // earlier errors are discarded
+        this.diagnosticCollection.clear();
+
+        filesDiag.forEach((issuesArray, fileName) =>
+          {
+            let fileURI = vscode.Uri.file(fileName);
+            this.diagnosticCollection.set(
+              fileURI,
+              issuesArray
+            );
+          }
+        );
       }
     );
   }