[LINT] Add rule explicit-begin

Add rule to check if the begin keyword always follows a if/else/for.
Related to chipsalliance#1321

verilog/analysis/checkers/BUILD

@@ -24,6 +24,7 @@ cc_library(
         ":disable-statement-rule",
         ":endif-comment-rule",
         ":enum-name-style-rule",
+        ":explicit-begin-rule",
         ":explicit-function-lifetime-rule",
         ":explicit-function-task-parameter-type-rule",
         ":explicit-parameter-storage-type-rule",
@@ -1249,6 +1250,38 @@ cc_test(
     ],
 )
 
+cc_library(
+    name = "explicit-begin-rule",
+    srcs = ["explicit_begin_rule.cc"],
+    hdrs = ["explicit_begin_rule.h"],
+    deps = [
+        "//common/analysis:lint-rule-status",
+        "//common/analysis:token-stream-lint-rule",
+        "//common/strings:comment-utils",
+        "//common/text:config-utils",
+        "//common/text:token-info",
+        "//verilog/analysis:descriptions",
+        "//verilog/analysis:lint-rule-registry",
+        "//verilog/parser:verilog-token-enum",
+        "@com_google_absl//absl/strings",
+    ],
+    alwayslink = 1,
+)
+
+cc_test(
+    name = "explicit-begin-rule_test",
+    srcs = ["explicit_begin_rule_test.cc"],
+    deps = [
+        ":explicit-begin-rule",
+        "//common/analysis:linter-test-utils",
+        "//common/analysis:token-stream-linter-test-utils",
+        "//common/text:symbol",
+        "//verilog/analysis:verilog-analyzer",
+        "//verilog/parser:verilog-token-enum",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
 cc_library(
     name = "explicit-function-lifetime-rule",
     srcs = ["explicit_function_lifetime_rule.cc"],

verilog/analysis/checkers/explicit_begin_rule.cc

@@ -0,0 +1,160 @@
+// Copyright 2017-2020 The Verible Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "verilog/analysis/checkers/explicit_begin_rule.h"
+
+#include <deque>
+#include <set>
+#include <stack>
+#include <string>
+
+#include "absl/strings/str_cat.h"
+#include "absl/strings/string_view.h"
+#include "common/analysis/lint_rule_status.h"
+#include "common/analysis/token_stream_lint_rule.h"
+#include "common/strings/comment_utils.h"
+#include "common/text/token_info.h"
+#include "verilog/analysis/descriptions.h"
+#include "verilog/analysis/lint_rule_registry.h"
+#include "verilog/parser/verilog_token_enum.h"
+
+namespace verilog {
+namespace analysis {
+
+using verible::AutoFix;
+using verible::LintRuleStatus;
+using verible::LintViolation;
+using verible::TokenInfo;
+using verible::TokenStreamLintRule;
+
+// Register the lint rule
+VERILOG_REGISTER_LINT_RULE(ExplicitBeginRule);
+
+static const char kMessage[] =
+    "All block construct shall explicitely use begin/end";
+
+const LintRuleDescriptor& ExplicitBeginRule::GetDescriptor() {
+  static const LintRuleDescriptor d{
+      .name = "explicit-begin",
+      .topic = "explicit-begin",
+      .desc =
+          "Checks that a Verilog ``begin`` directive follows all "
+          "if, else and for loops.",
+  };
+  return d;
+}
+
+void ExplicitBeginRule::HandleToken(const TokenInfo& token) {
+  // Responds to a token by updating the state of the analysis.
+  bool raise_violation = false;
+  switch (state_) {
+    case State::kNormal: {
+      // On if/else/for tokens;
+      // Also, skip all conditional statement after a for or a if
+      // Then, expect a `begin` token or record a violation.
+      switch (token.token_enum()) {
+        case TK_if:
+        case TK_for:
+          condition_expr_level_ = 0;
+          last_condition_start_ = token;
+          state_ = State::kInCondition;
+          break;
+        case TK_else:
+          last_condition_start_ = token;
+          end_of_condition_statement_ = token;
+          state_ = State::kInElse;
+          break;
+        default:
+          break;
+      }  // switch (token)
+      break;
+    }
+    case State::kInElse: {
+      // If we are in a else statement, we can either have a if (and need to
+      // skip cond. statement) Or directly wait for a begin. We make use of the
+      // boolean raise_violation in order to avoid code duplication.
+      switch (token.token_enum()) {
+        case TK_SPACE:  // stay in the same state
+          break;
+        case TK_COMMENT_BLOCK:
+        case TK_EOL_COMMENT:
+          break;
+        case TK_if:
+        case TK_for:
+          condition_expr_level_ = 0;
+          last_condition_start_ = token;
+          state_ = State::kInCondition;
+          break;
+        case TK_begin:
+          state_ = State::kNormal;
+          break;
+        default:
+          raise_violation = true;
+          break;
+      }  // switch (token)
+      break;
+    }
+    case State::kInCondition: {
+      if (token.text() == "(") {
+        condition_expr_level_++;
+      } else if (token.text() == ")") {
+        condition_expr_level_--;
+        if (condition_expr_level_ == 0) {
+          end_of_condition_statement_ = token;
+          state_ = State::kExpectBegin;
+        }
+      }
+      break;
+    }
+    case State::kExpectBegin: {
+      switch (token.token_enum()) {
+        case TK_SPACE:  // stay in the same state
+          break;
+        case TK_COMMENT_BLOCK:
+        case TK_EOL_COMMENT:
+          break;
+        case TK_begin:
+          // If we got our begin token, we go back to normal status
+          state_ = State::kNormal;
+          break;
+        default: {
+          raise_violation = true;
+          break;
+        }
+      }  // switch (token)
+      break;
+    }
+  }  // switch (state_)
+
+  if (raise_violation) {
+    violations_.insert(LintViolation(
+        last_condition_start_, absl::StrCat(kMessage),
+        {AutoFix(
+            "Insert begin",
+            {end_of_condition_statement_,
+             absl::StrCat(end_of_condition_statement_.text(), " begin")})}));
+
+    // Once the violation is raised, we go back to a normal, default, state
+    condition_expr_level_ = 0;
+    state_ = State::kNormal;
+    raise_violation = false;
+  }
+}
+
+LintRuleStatus ExplicitBeginRule::Report() const {
+  return LintRuleStatus(violations_, GetDescriptor());
+}
+
+}  // namespace analysis
+}  // namespace verilog

verilog/analysis/checkers/explicit_begin_rule.h

@@ -0,0 +1,88 @@
+// Copyright 2017-2020 The Verible Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef VERIBLE_VERILOG_ANALYSIS_CHECKERS_EXPLICIT_BEGIN_RULE_H_
+#define VERIBLE_VERILOG_ANALYSIS_CHECKERS_EXPLICIT_BEGIN_RULE_H_
+
+#include <set>
+#include <stack>
+#include <string>
+
+#include "common/analysis/lint_rule_status.h"
+#include "common/analysis/token_stream_lint_rule.h"
+#include "common/text/token_info.h"
+#include "verilog/analysis/descriptions.h"
+
+namespace verilog {
+namespace analysis {
+
+// Detects whether a Verilog `endif directive is followed by a comment that
+// matches the opening `ifdef or `ifndef.
+//
+// Accepted examples:
+//   `ifdef FOO
+//   `endif  // FOO
+//
+//   `ifndef BAR
+//   `endif  // BAR
+//
+// Rejected examples:
+//   `ifdef FOO
+//   `endif
+//
+//   `ifdef FOO
+//   `endif  // BAR
+//
+class ExplicitBeginRule : public verible::TokenStreamLintRule {
+ public:
+  using rule_type = verible::TokenStreamLintRule;
+
+  static const LintRuleDescriptor& GetDescriptor();
+
+  ExplicitBeginRule() : state_(State::kNormal), condition_expr_level_(0) {}
+
+  void HandleToken(const verible::TokenInfo& token) final;
+
+  verible::LintRuleStatus Report() const final;
+
+ private:
+  // States of the internal token-based analysis.
+  enum class State {
+    kNormal,
+    kInCondition,
+    kInElse,
+    kExpectBegin
+  };
+
+  // Internal lexical analysis state.
+  State state_;
+
+  // Level of nested parenthesis when analizing conditional expressions.
+  int condition_expr_level_;
+
+  // Token information for the last seen block opening (for/if/else).
+  verible::TokenInfo last_condition_start_ = verible::TokenInfo::EOFToken();
+  verible::TokenInfo end_of_condition_statement_ = verible::TokenInfo::EOFToken();
+
+
+  // Collection of found violations.
+  std::set<verible::LintViolation> violations_;
+
+  void trigger_violation_();
+};
+
+}  // namespace analysis
+}  // namespace verilog
+
+#endif  // VERIBLE_VERILOG_ANALYSIS_CHECKERS_ENDIF_COMMENT_RULE_H_