How to use rule for having a begin and end

[muneebullashariff]

Summary

I want to use the rule for checking if there are any if-else or for or while with single statements but doesn't have begin-end.

Which rules should I use, as I don't see them in the rules list.

Proposal

[optional] Suggest approach or include pseudo code for how one might implement
the rule.

Additional context

https://chipsalliance.github.io/verible/lint.html

[IEncinas10]

Indeed, there is no rule that checks this. It shouldn't be too hard to create it, if you're willing to spend some time on it.

Anyway, thanks for the sugestion. I might give it a go in a couple of months.

[muneebullashariff]

If you can guide me, I would like to add this rule and maybe be a contributor. Would you mentor me in this regard?

[IEncinas10]

Disclaimer: I'm not a maintainer, but I have done a couple of PRs (mainly lint rule stuff). But happy to help you!

When I did my first PR I found the documentation pretty decent, so you might want to start there: https://github.com/chipsalliance/verible/blob/master/doc/style_lint.md. I'll leave you a couple of example PRs I have done in case they're useful to you.

Basically, what you have to do is create a new SyntaxTreeLintRule (see documentation linked above) that matches ifs, elses, ... that don't have the begin ... end.

Then, what I would do is use verible-syntax-tree -printtree on a example verilog file that shows the patterns you want to catch, so that you can understand the structure and match it in the rule.

Using:

// saved in a file named "beginend.sv"
module beginend ();

  initial begin

    if (1) begin
    end else begin
    end

    if (1) x = y;
    else y = x;

  end
endmodule

verible-verilog-syntax --printtree beginend.sv

Will give you the syntax tree, and you have to see how the pattern you want to discourage is structured.

From my example, it looks like the if with begin ... end will have a kIfBody whose only direct child is a kSeqBlock

Node @1 (tag: kIfBody) {
  Node @0 (tag: kSeqBlock) {
    Node @0 (tag: kBegin) {
      Leaf @0 (#"begin" @49-54: "begin")
    }
    Node @1 (tag: kBlockItemStatementList) {
    }
    Node @2 (tag: kEnd) {
      Leaf @0 (#"end" @59-62: "end")
    }
  }
}

But the "wrong" if will not have this kSeqBlock as a child

Node @1 (tag: kIfBody) {
  Node @0 (tag: kNetVariableAssignment) {
    Node @0 (tag: kLPValue) {
      Node @0 (tag: kReference) {
        Node @0 (tag: kLocalRoot) {
          Node @0 (tag: kUnqualifiedId) {
            Leaf @0 (#SymbolIdentifier @94-95: "x")
          }
        }
      }
    }
    Leaf @1 (#'=' @96-97: "=")
    Node @2 (tag: kExpression) {
      Node @0 (tag: kFunctionCall) {
        Node @0 (tag: kReference) {
          Node @0 (tag: kLocalRoot) {
            Node @0 (tag: kUnqualifiedId) {
              Leaf @0 (#SymbolIdentifier @98-99: "y")
            }
          }
        }
      }
    }
    Leaf @3 (#';' @99-100: ";")
  }
}

Other information

If you haven't, check https://github.com/chipsalliance/verible/tree/master#developers-welcome

Bazel specific

I hadn't used bazel before and I remember being a bit lost at the begining so, just in case it is helpful:

• You have to modify the BUILD file in verilog/analysis/checkers (see the PRs I linked)
• Then, you will be able to test it with bazel test //verilog/analysis/checkers:<however-you-decide-to-name-this-rule>

Extra: I usually rebuild verible-verilog-ls and run the modified version locally to test how the rule behaves directly in the editor. This is also helpful if you plan to provide autofixes.

Examples

1. Add new lint rule for negative array dimensions #1835 (a bit rough on my part, but you might find useful the help I got from the maintainers and maybe avoid a couple of basic errors I did)
2. linter: Add suspicious-semicolon rule #2016

[sconwayaus]

FWIW: there's an old PR that hasn't closed here #1336.