Merge pull request #2212 from sconwayaus/macro_name_style_regex

Adding regex configuration for macro names

verilog/analysis/checkers/BUILD

@@ -1568,15 +1568,17 @@ cc_library(
     deps = [
         "//common/analysis:lint-rule-status",
         "//common/analysis:token-stream-lint-rule",
-        "//common/strings:naming-utils",
+        "//common/text:config-utils",
         "//common/text:token-info",
         "//verilog/analysis:descriptions",
         "//verilog/analysis:lint-rule-registry",
         "//verilog/parser:verilog-lexer",
         "//verilog/parser:verilog-token-classifications",
         "//verilog/parser:verilog-token-enum",
+        "@com_google_absl//absl/status",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:string_view",
+        "@com_googlesource_code_re2//:re2",
     ],
     alwayslink = 1,
 )

verilog/analysis/checkers/macro_name_style_rule.cc

@@ -14,14 +14,18 @@
 
 #include "verilog/analysis/checkers/macro_name_style_rule.h"
 
+#include <memory>
 #include <set>
+#include <string>
 
+#include "absl/status/status.h"
 #include "absl/strings/match.h"
+#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/naming_utils.h"
+#include "common/text/config_utils.h"
 #include "common/text/token_info.h"
+#include "re2/re2.h"
 #include "verilog/analysis/descriptions.h"
 #include "verilog/analysis/lint_rule_registry.h"
 #include "verilog/parser/verilog_lexer.h"
@@ -31,29 +35,52 @@
 namespace verilog {
 namespace analysis {
 
+VERILOG_REGISTER_LINT_RULE(MacroNameStyleRule);
+
 using verible::LintRuleStatus;
 using verible::LintViolation;
 using verible::TokenInfo;
-using verible::TokenStreamLintRule;
 
-// Register the lint rule
-VERILOG_REGISTER_LINT_RULE(MacroNameStyleRule);
+static constexpr absl::string_view kUVMLowerCaseMessage =
+    "'uvm_*' named macros must follow 'lower_snake_case' format.";
+
+static constexpr absl::string_view kUVMUpperCaseMessage =
+    "'UVM_*' named macros must follow 'UPPER_SNAKE_CASE' format.";
+
+static absl::string_view lower_snake_case_regex = "[a-z_0-9]+";
+static absl::string_view upper_snake_case_regex = "[A-Z_0-9]+";
 
-static constexpr absl::string_view kMessage =
-    "Macro names must contain only CAPITALS, underscores, and digits.  "
-    "Exception: UVM-like macros.";
+MacroNameStyleRule::MacroNameStyleRule()
+    : style_regex_(
+          std::make_unique<re2::RE2>(upper_snake_case_regex, re2::RE2::Quiet)),
+      style_lower_snake_case_regex_(
+          std::make_unique<re2::RE2>(lower_snake_case_regex, re2::RE2::Quiet)),
+      style_upper_snake_case_regex_(std::make_unique<re2::RE2>(
+          upper_snake_case_regex, re2::RE2::Quiet)) {}
 
 const LintRuleDescriptor &MacroNameStyleRule::GetDescriptor() {
   static const LintRuleDescriptor d{
       .name = "macro-name-style",
       .topic = "defines",
       .desc =
-          "Checks that every macro name follows ALL_CAPS naming convention. "
-          "_Exception_: UVM-like macros.",
+          "Checks that macro names conform to a naming convention defined by a "
+          "RE2 regular expression. The default regex pattern expects "
+          "\"UPPER_SNAKE_CASE\". Exceptions are made for UVM like macros, "
+          "where macros named 'uvm_*' and 'UVM_*' follow \"lower_snake_case\" "
+          "and \"UPPER_SNAKE_CASE\" naming conventions respectively. Refer to "
+          "https://github.com/chipsalliance/verible/tree/master/verilog/tools/"
+          "lint#readme for more detail on verible regex patterns.",
+      .param = {{"style_regex", std::string(upper_snake_case_regex),
+                 "A regex used to check macro names style."}},
   };
   return d;
 }
 
+std::string MacroNameStyleRule::CreateViolationMessage() {
+  return absl::StrCat("Macro name does not match the naming convention ",
+                      "defined by regex pattern: ", style_regex_->pattern());
+}
+
 void MacroNameStyleRule::HandleToken(const TokenInfo &token) {
   const auto token_enum = static_cast<verilog_tokentype>(token.token_enum());
   const absl::string_view text(token.text());
@@ -84,19 +111,19 @@ void MacroNameStyleRule::HandleToken(const TokenInfo &token) {
         case PP_Identifier: {
           if (absl::StartsWith(text, "uvm_")) {
             // Special case for uvm_* macros
-            if (!verible::IsLowerSnakeCaseWithDigits(text)) {
-              violations_.insert(LintViolation(token, kMessage));
+            if (!RE2::FullMatch(text, *style_lower_snake_case_regex_)) {
+              violations_.insert(LintViolation(token, kUVMLowerCaseMessage));
             }
           } else if (absl::StartsWith(text, "UVM_")) {
             // Special case for UVM_* macros
-            if (!verible::IsNameAllCapsUnderscoresDigits(text)) {
-              violations_.insert(LintViolation(token, kMessage));
+            if (!RE2::FullMatch(text, *style_upper_snake_case_regex_)) {
+              violations_.insert(LintViolation(token, kUVMUpperCaseMessage));
             }
           } else {
             // General case for everything else
-            // TODO(fangism): make this configurable
-            if (!verible::IsNameAllCapsUnderscoresDigits(text)) {
-              violations_.insert(LintViolation(token, kMessage));
+            if (!RE2::FullMatch(text, *style_regex_)) {
+              violations_.insert(
+                  LintViolation(token, CreateViolationMessage()));
             }
           }
           state_ = State::kNormal;
@@ -109,6 +136,13 @@ void MacroNameStyleRule::HandleToken(const TokenInfo &token) {
   }  // switch (state_)
 }
 
+absl::Status MacroNameStyleRule::Configure(absl::string_view configuration) {
+  using verible::config::SetRegex;
+  absl::Status s = verible::ParseNameValues(
+      configuration, {{"style_regex", SetRegex(&style_regex_)}});
+  return s;
+}
+
 LintRuleStatus MacroNameStyleRule::Report() const {
   return LintRuleStatus(violations_, GetDescriptor());
 }

verilog/analysis/checkers/macro_name_style_rule.h

@@ -15,30 +15,40 @@
 #ifndef VERIBLE_VERILOG_ANALYSIS_CHECKERS_MACRO_NAME_STYLE_RULE_H_
 #define VERIBLE_VERILOG_ANALYSIS_CHECKERS_MACRO_NAME_STYLE_RULE_H_
 
+#include <memory>
 #include <set>
+#include <string>
 
+#include "absl/status/status.h"
+#include "absl/strings/string_view.h"
 #include "common/analysis/lint_rule_status.h"
 #include "common/analysis/token_stream_lint_rule.h"
 #include "common/text/token_info.h"
+#include "re2/re2.h"
 #include "verilog/analysis/descriptions.h"
 
 namespace verilog {
 namespace analysis {
 
-// MacroNameStyleRule checks that every macro name contains only all capital
-// letters, underscores, and digits.
+// MacroNameStyleRule checks that macro names follow
+// a naming convention matching a regex pattern. Exceptions
+// are made for uvm_* and UVM_* named macros.
 class MacroNameStyleRule : public verible::TokenStreamLintRule {
  public:
   using rule_type = verible::TokenStreamLintRule;
 
+  MacroNameStyleRule();
+
   static const LintRuleDescriptor &GetDescriptor();
 
-  MacroNameStyleRule() = default;
+  std::string CreateViolationMessage();
 
   void HandleToken(const verible::TokenInfo &token) final;
 
   verible::LintRuleStatus Report() const final;
 
+  absl::Status Configure(absl::string_view configuration) final;
+
  private:
   // States of the internal token-based analysis.
   enum class State {
@@ -50,6 +60,11 @@ class MacroNameStyleRule : public verible::TokenStreamLintRule {
   State state_ = State::kNormal;
 
   std::set<verible::LintViolation> violations_;
+
+  // A regex to check the style against
+  std::unique_ptr<re2::RE2> style_regex_;
+  std::unique_ptr<re2::RE2> style_lower_snake_case_regex_;
+  std::unique_ptr<re2::RE2> style_upper_snake_case_regex_;
 };
 
 }  // namespace analysis

verilog/analysis/checkers/macro_name_style_rule_test.cc

@@ -27,6 +27,7 @@ namespace analysis {
 namespace {
 
 using verible::LintTestCase;
+using verible::RunConfiguredLintTestCases;
 using verible::RunLintTestCases;
 
 // Tests that the expected number of MacroNameStyleRule violations are found.
@@ -73,6 +74,19 @@ TEST(MacroNameStyleRuleTest, BasicTests) {
   RunLintTestCases<VerilogAnalyzer, MacroNameStyleRule>(kTestCases);
 }
 
+TEST(MacroNameStyleRuleTest, LowerSnakeCaseTests) {
+  const std::initializer_list<LintTestCase> kTestCases = {
+      {""},
+      {"`define foo 1"},
+      {"`define foo 1\n"},
+      // special case uvm_* macros
+      {"`define uvm_foo_bar(arg) arg\n"},
+      {"`define UVM_FOO_BAR(arg) arg\n"},
+  };
+  RunConfiguredLintTestCases<VerilogAnalyzer, MacroNameStyleRule>(
+      kTestCases, "style_regex:[a-z_0-9]+");
+}
+
 }  // namespace
 }  // namespace analysis
 }  // namespace verilog