chipsalliance / verible

Verible is a suite of SystemVerilog developer tools, including a parser, style-linter, formatter and language server
https://chipsalliance.github.io/verible/
Other
1.26k stars 198 forks source link

Support for structured output for the style linter #257

Open msfschaffner opened 4 years ago

msfschaffner commented 4 years ago

In order to build the OpenTitan style lint dashboard we currently parse the output from Verible, and convert it into a very simple hjson file that our dashboard tool then consumes:

{
  tool: veriblelint
  errors: []
  warnings: []
  lint_errors: []
  lint_warnings: []
  lint_infos: []
}

We use the same hjson file and a similar parser script to capture output from the AscentLint tool.

Link to Verible parser script Link to Verible dashboard Link to AscentLint dashboard

The fields errors and warnings contain file IO messages or messages output by the tool itself, whereas the fields prefixed with lint_ contain lint-related messages. In this case, we just dump all Verible lint messages into lint_warnings.

Native support for such a structured output file in Verible would obviate the need for this extra parsing step, would make the process more robust and also allow for more convenient integration into other dashboard or CI flows.

The hjson structure mentioned above should just serve as a simple example, and we would be happy to adjust our dashboard tool to accept a modified version thereof with similar / more fields.

msfschaffner commented 4 years ago

b/153481529

fangism commented 4 years ago

Note that at the moment we don't have a notion of severity for linter findings. It might not make sense to slap on a configurable severity level per rule, because a single rule could yield multiple classes of findings.

msfschaffner commented 4 years ago

IMO it's fine if they are all just in the lint warnings category for now. you may want to consider putting syntax / file IO errors preventing the linter from running into the errors list.

wallento commented 4 years ago

Hi, I have just recently started to look into the same topic for Verilator. Currently json and xml are the front runners, with a slight tendency to json. It would be great if both tools use same type and structure. My target is the new Github Action I am building for SV Linters, that I for example integrated here: https://github.com/lowRISC/ibex/pull/960 I started playing around with formats and have a small package ready similar to what @msfschaffner did: https://github.com/librecores/eda-log-parser I hacked [veriblelint, verilator, vivado] -> [azure, dict, ghaction, json] into it as a start. I will probably use it for transforms in the long run, but having structured logs is better probably. So, probably there will be json as input too and the outputs are annotations for Azure and Github Actions right now, which I see as a key element. If you look at the output json its very boilerplate as you would expect:

$ cat verible.log
../../../rtl/ibex_tracer_pkg.sv:14:45: Binary literal 25\'b? is shorter than its declared width: 25. [Style: number-literals] [undersized-binary-literal]
../../../rtl/ibex_tracer_pkg.sv:15:45: Binary literal 25\'b? is shorter than its declared width: 25. [Style: number-literals] [undersized-binary-literal]
../../../rtl/ibex_alu.sv:713:45: When an unpacked dimension range is zero-based ([0:N-1]), declare size as [N] instead. [Style: unpacked-ordering] [unpacked-dimensions-range-ordering]
../../../rtl/ibex_core.sv:992:38: Unpacked dimension range must be declared in big-endian ([0:N-1]) order.  Declare zero-based big-endian unpacked dimensions sized as [N]. [Style: unpacked-ordering] [unpacked-dimensions-range-ordering]
$ cat verible.log | eda-log-parser -t veriblelint -f json
[
    {
        "severity": "warning",
        "msg": "Binary literal 25\\'b? is shorter than its declared width: 25.",
        "file": "../../../rtl/ibex_tracer_pkg.sv",
        "line": "14",
        "col": "45",
        "code": "number-literals"
    },
    {
        "severity": "warning",
        "msg": "Binary literal 25\\'b? is shorter than its declared width: 25.",
        "file": "../../../rtl/ibex_tracer_pkg.sv",
        "line": "15",
        "col": "45",
        "code": "number-literals"
    },
    {
        "severity": "warning",
        "msg": "When an unpacked dimension range is zero-based ([0:N-1]), declare size as [N] instead.",
        "file": "../../../rtl/ibex_alu.sv",
        "line": "713",
        "col": "45",
        "code": "unpacked-ordering"
    },
    {
        "severity": "warning",
        "msg": "Unpacked dimension range must be declared in big-endian ([0:N-1]) order.  Declare zero-based big-endian unpacked dimensions sized as [N].",
        "file": "../../../rtl/ibex_core.sv",
        "line": "992",
        "col": "38",
        "code": "unpacked-ordering"
    }
]
$ cat verible.log | eda-log-parser -t veriblelint -f ghaction
::warning file=../../../rtl/ibex_tracer_pkg.sv,line=14,col=45::Binary literal 25'b? is shorter than its declared width: 25. (number-literals)
::warning file=../../../rtl/ibex_tracer_pkg.sv,line=15,col=45::Binary literal 25'b? is shorter than its declared width: 25. (number-literals)
::warning file=../../../rtl/ibex_alu.sv,line=713,col=45::When an unpacked dimension range is zero-based ([0:N-1]), declare size as [N] instead. (unpacked-ordering)
::warning file=../../../rtl/ibex_core.sv,line=992,col=38::Unpacked dimension range must be declared in big-endian ([0:N-1]) order.  Declare zero-based big-endian unpacked dimensions sized as [N]. (unpacked-ordering)

This is meant as a playground for formats and start of further discussion. Cheers, Stefan