From e876532aa15653cf4b5a301ccab888a2eff01128 Mon Sep 17 00:00:00 2001 From: alx_the_new_guy Date: Wed, 24 Jul 2024 15:29:52 +0300 Subject: [PATCH 1/2] Improved error parsing when using verilator --- src/linter/VerilatorLinter.ts | 64 +++++++++++++++++++++++++++-------- 1 file changed, 50 insertions(+), 14 deletions(-) diff --git a/src/linter/VerilatorLinter.ts b/src/linter/VerilatorLinter.ts index 7581148b..985a668f 100644 --- a/src/linter/VerilatorLinter.ts +++ b/src/linter/VerilatorLinter.ts @@ -107,42 +107,78 @@ export default class VerilatorLinter extends BaseLinter { command, { cwd: cwd }, (_error: Error, _stdout: string, stderr: string) => { - let diagnostics: vscode.Diagnostic[] = []; + //let diagnostics: vscode.Diagnostic[] = []; + + // basically DiagnosticsCollection but with ability to append diag lists + let filesDiag = {}; + let error_warning_counter = 0; 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; - } - if (!line.startsWith('%') || line.indexOf(docUri) <= 0) { + if (!line.startsWith('%')) { + this.logger.error(line); return; } + // for this regex, match sections are: + // 1 - severity (warning/error) + // 3 - error/warning code (EOFNEWLINE, ...), + // 5 - file path + // 9 - line number + // 11 - column number + // 12 - message + let rex = line.match( - /%(\w+)(-[A-Z0-9_]+)?:\s*(\w+:)?(?:[^:]+):\s*(\d+):(?:\s*(\d+):)?\s*(\s*.+)/ + /(\w+)(-([A-Z0-9]+))?: ((\S+((\.sv)|(\.v))):(\d+):((\d+):)? )?(.*)/ ); + // vscode problems are tied to files, so if there is no file name, no point adding + if (!rex[5]) {return;} + + // replacing "\\" and "\" with "/" for consistency + if (isWindows) + { + rex[5] = rex[5].replace(/(\\\\)|(\\)/, "/"); + } + + // if no errors for this file, new list needs to be created + if (!(rex[5] in Object.keys(filesDiag))) + { + filesDiag[rex[5]] = []; + } + + if (rex && rex[0].length > 0) { - let lineNum = Number(rex[4]) - 1; - let colNum = Number(rex[5]) - 1; + let lineNum = Number(rex[9]) - 1; + let colNum = Number(rex[11]) - 1; // Type of warning is in rex[2] colNum = isNaN(colNum) ? 0 : colNum; // for older Verilator versions (< 4.030 ~ish) if (!isNaN(lineNum)) { - diagnostics.push({ + filesDiag[rex[5]].push({ severity: this.convertToSeverity(rex[1]), range: new vscode.Range(lineNum, colNum, lineNum, Number.MAX_VALUE), - message: rex[6], - code: 'verilator', + message: rex[12], + code: rex[3], source: 'verilator', }); + + error_warning_counter++; } 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); + this.logger.info(`[verilator] ${error_warning_counter} errors/warnings returned`); + this.diagnosticCollection.clear() + for (let fileName in filesDiag) + { + let fileURI = vscode.Uri.file(fileName); + // adding diag info for each file + this.diagnosticCollection.set( + fileURI, + filesDiag[fileName] + ); + } } ); } From 6715f280db6e89ab3039a9adf3466a94a2e47960 Mon Sep 17 00:00:00 2001 From: alx_the_new_guy Date: Sat, 27 Jul 2024 11:23:53 +0300 Subject: [PATCH 2/2] Major: stderr passthrough Improved problem matcher regex. Now importaint sections are named Added better tagging for output blocks. Added changes to changelog Smaller stuff: changed container for messages to be a Map fixed regex that replaced slashes in windows paths removed old code --- CHANGELOG.md | 12 ++- src/linter/VerilatorLinter.ts | 154 +++++++++++++++++++++++++--------- 2 files changed, 125 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 830d3cf5..83e2b319 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/linter/VerilatorLinter.ts b/src/linter/VerilatorLinter.ts index 985a668f..d393730d 100644 --- a/src/linter/VerilatorLinter.ts +++ b/src/linter/VerilatorLinter.ts @@ -103,82 +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[] = []; // basically DiagnosticsCollection but with ability to append diag lists - let filesDiag = {}; - let error_warning_counter = 0; - stderr.split(/\r?\n/g).forEach((line, _) => { + 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; + } + } + // 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('%')) { - this.logger.error(line); + + // allows for persistent + if (lastDiagMessageType === 'Warning') { this.logger.warn(line); } + else { this.logger.error(line); } return; } - // for this regex, match sections are: - // 1 - severity (warning/error) - // 3 - error/warning code (EOFNEWLINE, ...), - // 5 - file path - // 9 - line number - // 11 - column number - // 12 - message - let rex = line.match( - /(\w+)(-([A-Z0-9]+))?: ((\S+((\.sv)|(\.v))):(\d+):((\d+):)? )?(.*)/ + // 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( + /%(?\w+)/.source + // matches "%Warning" or "%Error" + + // this matches errorcode with "-" before it, but the "-" doesn't go into ErrorCode match group + /(-(?[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 + + /((?(\S| )+(?(\.svh)|(\.sv)|(\.SV)|(\.vh)|(\.vl)|(\.v))):((?\d+):)?((?\d+):)? )?/.source + + + // matches error message produced by Verilator + /(?.*)/.source + , "g" ); - // vscode problems are tied to files, so if there is no file name, no point adding - if (!rex[5]) {return;} + 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[5] = rex[5].replace(/(\\\\)|(\\)/, "/"); + rex.groups["filePath"] = rex.groups["filePath"].replace(/(\\\\)|(\\)/g, "/"); } - // if no errors for this file, new list needs to be created - if (!(rex[5] in Object.keys(filesDiag))) + // if there isn't a list of errors for this file already, it + // needs to be created + if (!filesDiag.has(rex.groups["filePath"])) { - filesDiag[rex[5]] = []; + filesDiag.set(rex.groups["filePath"], []); } if (rex && rex[0].length > 0) { - let lineNum = Number(rex[9]) - 1; - let colNum = Number(rex[11]) - 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)) { - filesDiag[rex[5]].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[12], - code: rex[3], + message: rex.groups["verboseError"], + code: rex.groups["errorCode"], source: 'verilator', }); - error_warning_counter++; } return; } - this.logger.warn('[verilator] failed to parse error: ' + line); }); - this.logger.info(`[verilator] ${error_warning_counter} errors/warnings returned`); - this.diagnosticCollection.clear() - for (let fileName in filesDiag) - { - let fileURI = vscode.Uri.file(fileName); - // adding diag info for each file - this.diagnosticCollection.set( - fileURI, - filesDiag[fileName] - ); - } + + // 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 + ); + } + ); } ); }