PR #308: Initial implementation of what could be a solution to #281.

UvmMacroSemicolonRule checks if there are any unwanted ';' after a `uvm_* macro call. Also, a new method has been added in CST/macro.h that makes rule implementation a bit cleaner. GitHub PR #308 Copybara import of the project: - 1975dbc Initial implementation of what could be a solution to #281. by Ciprian A <[email protected]> - 77c1a18 Changing rule strategy and adding test improvements. by Ciprian A <[email protected]> - 2c6bc69 Adding a simple integration test and some small things I ... by Ciprian A <[email protected]> Closes #308 Fixes #281 PiperOrigin-RevId: 314202780
chipsalliance · Jun 2, 2020 · 5383335 · 5383335
1 parent 85f8ff8
commit 5383335
Show file tree

Hide file tree

Showing 10 changed files with 567 additions and 3 deletions.
diff --git a/verilog/CST/context_functions.h b/verilog/CST/context_functions.h
@@ -58,6 +58,11 @@ inline bool ContextIsInsideTaskFunctionPortList(
   return context.IsInside(NodeEnum::kPortList);
 }
 
+inline bool ContextIsInsideStatement(
+    const verible::SyntaxTreeContext& context) {
+  return context.IsInside(NodeEnum::kStatement);
+}
+
 // add more as needed
 
 }  // namespace analysis

diff --git a/verilog/CST/macro.cc b/verilog/CST/macro.cc
@@ -44,11 +44,13 @@ const TokenInfo& GetMacroGenericItemId(const Symbol& s) {
   return GetSubtreeAsLeaf(s, NodeEnum::kMacroGenericItem, 0).get();
 }
 
+const SyntaxTreeNode& GetMacroCallParenGroup(const Symbol& s) {
+  return GetSubtreeAsNode(s, NodeEnum::kMacroCall, 1, NodeEnum::kParenGroup);
+}
+
 const SyntaxTreeNode& GetMacroCallArgs(const Symbol& s) {
-  const auto& paren_group_node =
-      GetSubtreeAsNode(s, NodeEnum::kMacroCall, 1, NodeEnum::kParenGroup);
   // See structure of (CST) MakeParenGroup().
-  return GetSubtreeAsNode(paren_group_node, NodeEnum::kParenGroup, 1,
+  return GetSubtreeAsNode(GetMacroCallParenGroup(s), NodeEnum::kParenGroup, 1,
                           NodeEnum::kMacroArgList);
 }
 

diff --git a/verilog/CST/macro.h b/verilog/CST/macro.h
@@ -41,6 +41,9 @@ const verible::TokenInfo& GetMacroCallId(const verible::Symbol&);
 // Returns the leaf containing the macro (as generic item) name.
 const verible::TokenInfo& GetMacroGenericItemId(const verible::Symbol&);
 
+// Returns the node containing the macro call paren group
+const verible::SyntaxTreeNode& GetMacroCallParenGroup(const verible::Symbol& s);
+
 // Returns the node containing the macro call arguments (without parentheses).
 const verible::SyntaxTreeNode& GetMacroCallArgs(const verible::Symbol&);
 

diff --git a/verilog/analysis/checkers/BUILD b/verilog/analysis/checkers/BUILD
@@ -55,6 +55,7 @@ cc_library(
         ":struct_union_name_style_rule",
         ":undersized_binary_literal_rule",
         ":unpacked_dimensions_rule",
+        ":uvm_macro_semicolon_rule",
         ":v2001_generate_begin_rule",
         ":void_cast_rule",
     ],
@@ -1679,3 +1680,39 @@ cc_test(
         "@com_google_googletest//:gtest_main",
     ],
 )
+
+cc_library(
+    name = "uvm_macro_semicolon_rule",
+    srcs = ["uvm_macro_semicolon_rule.cc"],
+    hdrs = ["uvm_macro_semicolon_rule.h"],
+    deps = [
+        "//common/analysis:citation",
+        "//common/analysis:lint_rule_status",
+        "//common/analysis:syntax_tree_lint_rule",
+        "//common/text:concrete_syntax_leaf",
+        "//common/text:symbol",
+        "//common/text:syntax_tree_context",
+        "//common/text:token_info",
+        "//verilog/CST:context_functions",
+        "//verilog/CST:macro",
+        "//verilog/CST:verilog_matchers",
+        "//verilog/analysis:descriptions",
+        "//verilog/analysis:lint_rule_registry",
+        "@com_google_absl//absl/strings",
+    ],
+    alwayslink = 1,
+)
+
+cc_test(
+    name = "uvm_macro_semicolon_rule_test",
+    srcs = ["uvm_macro_semicolon_rule_test.cc"],
+    deps = [
+        ":uvm_macro_semicolon_rule",
+        "//common/analysis:linter_test_utils",
+        "//common/analysis:syntax_tree_linter_test_utils",
+        "//common/text:symbol",
+        "//verilog/CST:verilog_nonterminals",
+        "//verilog/analysis:verilog_analyzer",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
diff --git a/verilog/analysis/checkers/uvm_macro_semicolon_rule.cc b/verilog/analysis/checkers/uvm_macro_semicolon_rule.cc
@@ -0,0 +1,109 @@
+// 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/uvm_macro_semicolon_rule.h"
+
+#include <string>
+
+#include "absl/strings/match.h"
+#include "absl/strings/str_cat.h"
+#include "common/analysis/citation.h"
+#include "common/analysis/lint_rule_status.h"
+#include "common/text/concrete_syntax_leaf.h"
+#include "common/text/symbol.h"
+#include "common/text/syntax_tree_context.h"
+#include "common/text/token_info.h"
+#include "verilog/CST/context_functions.h"
+#include "verilog/CST/macro.h"
+#include "verilog/analysis/descriptions.h"
+#include "verilog/analysis/lint_rule_registry.h"
+
+namespace verilog {
+namespace analysis {
+
+using verible::GetVerificationCitation;
+using verible::LintViolation;
+
+// Register UvmMacroSemicolonRule
+VERILOG_REGISTER_LINT_RULE(UvmMacroSemicolonRule);
+
+absl::string_view UvmMacroSemicolonRule::Name() {
+  return "uvm-macro-semicolon";
+}
+const char UvmMacroSemicolonRule::kTopic[] = "uvm-macro-semicolon-convention";
+// TODO(b/155128436): verify style guide anchor name
+
+std::string UvmMacroSemicolonRule::GetDescription(
+    DescriptionType description_type) {
+  return absl::StrCat("Checks that no `uvm_* macro calls end with ';'. See ",
+                      GetVerificationCitation(kTopic), ".");
+}
+
+// Returns a diagnostic message for this lint violation.
+static std::string FormatReason(const verible::TokenInfo& macro_id) {
+  return absl::StrCat("UVM macro call, ", macro_id.text,
+                      " should not be followed by a semicolon \';\'.");
+}
+
+// Returns true if leaf is a macro and matches `uvm_
+static bool IsUvmMacroId(const verible::SyntaxTreeLeaf& leaf) {
+  if (leaf.Tag().tag == verilog_tokentype::MacroCallId ||
+      leaf.Tag().tag == verilog_tokentype::MacroIdItem ||
+      leaf.Tag().tag == verilog_tokentype::MacroIdentifier) {
+    return absl::StartsWithIgnoreCase(leaf.get().text, "`uvm_");
+  }
+  return false;
+}
+
+void UvmMacroSemicolonRule::HandleLeaf(
+    const verible::SyntaxTreeLeaf& leaf,
+    const verible::SyntaxTreeContext& context) {
+  if (ContextIsInsideStatement(context) ||
+      context.IsInside(NodeEnum::kMacroCall) ||
+      context.IsInside(NodeEnum::kDataDeclaration)) {
+    switch (state_) {
+      case State::kNormal: {
+        if (IsUvmMacroId(leaf)) {
+          macro_id_ = leaf.get();
+          state_ = State::kCheckMacro;
+        }
+        break;
+      }
+
+      case State::kCheckMacro: {
+        if (leaf.Tag().tag == ';') {
+          violations_.insert(
+              LintViolation(leaf, FormatReason(macro_id_), context));
+          state_ = State::kNormal;
+        } else if (leaf.Tag().tag ==
+                   verilog_tokentype::MacroCallCloseToEndLine) {
+          state_ = State::kNormal;
+        }
+        break;
+      }
+    }
+  } else {
+    state_ = State::kNormal;
+    macro_id_ = verible::TokenInfo::EOFToken();
+    return;
+  }
+}
+
+verible::LintRuleStatus UvmMacroSemicolonRule::Report() const {
+  return verible::LintRuleStatus(violations_, Name(),
+                                 GetVerificationCitation(kTopic));
+}
+
+}  // namespace analysis
+}  // namespace verilog
diff --git a/verilog/analysis/checkers/uvm_macro_semicolon_rule.h b/verilog/analysis/checkers/uvm_macro_semicolon_rule.h
@@ -0,0 +1,79 @@
+// 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_UVM_MACRO_SEMICOLON_RULE_H_
+#define VERIBLE_VERILOG_ANALYSIS_CHECKERS_UVM_MACRO_SEMICOLON_RULE_H_
+
+#include <string>
+
+#include "common/analysis/lint_rule_status.h"
+#include "common/analysis/syntax_tree_lint_rule.h"
+#include "common/text/concrete_syntax_leaf.h"
+#include "common/text/symbol.h"
+#include "common/text/syntax_tree_context.h"
+#include "verilog/CST/verilog_matchers.h"
+#include "verilog/analysis/descriptions.h"
+
+namespace verilog {
+namespace analysis {
+
+// UvmMacroSemicolonRule checks for `uvm_* macro calls that end with ';'
+//
+// Example violations:
+// class Bad;
+//   function foo();
+//     `uvm_error("id","message");
+//     `uvm_error("id",
+//                "message");
+//    endfunction
+// endclass
+
+class UvmMacroSemicolonRule : public verible::SyntaxTreeLintRule {
+ public:
+  using rule_type = verible::SyntaxTreeLintRule;
+  static absl::string_view Name();
+
+  UvmMacroSemicolonRule() : state_(State::kNormal) {}
+
+  // Returns the description of the rule implemented
+  static std::string GetDescription(DescriptionType);
+
+  void HandleLeaf(const verible::SyntaxTreeLeaf& leaf,
+                  const verible::SyntaxTreeContext& context) override;
+
+  verible::LintRuleStatus Report() const override;
+
+ private:
+  // States of the internal leaf-based analysis.
+  enum class State {
+    kNormal,
+    kCheckMacro,
+  };
+
+  // Internal analysis state
+  State state_;
+
+  // Save the matching macro and include it in diagnostic message
+  verible::TokenInfo macro_id_ = verible::TokenInfo::EOFToken();
+
+  // Link to style-guide rule.
+  static const char kTopic[];
+
+  std::set<verible::LintViolation> violations_;
+};
+
+}  // namespace analysis
+}  // namespace verilog
+
+#endif  // VERIBLE_VERILOG_ANALYSIS_CHECKERS_UVM_MACRO_SEMICOLON_RULE_H_