Merge pull request #2211 from sconwayaus/interface_name_style_regex

Added regex configuration to the interface name rule

verilog/analysis/checkers/BUILD

@@ -2025,7 +2025,7 @@ cc_library(
         "//common/analysis:syntax-tree-lint-rule",
         "//common/analysis/matcher",
         "//common/analysis/matcher:bound-symbol-manager",
-        "//common/strings:naming-utils",
+        "//common/text:config-utils",
         "//common/text:symbol",
         "//common/text:syntax-tree-context",
         "//common/text:token-info",
@@ -2034,8 +2034,10 @@ cc_library(
         "//verilog/CST:verilog-matchers",
         "//verilog/analysis:descriptions",
         "//verilog/analysis:lint-rule-registry",
+        "@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/interface_name_style_rule.cc

@@ -14,17 +14,21 @@
 
 #include "verilog/analysis/checkers/interface_name_style_rule.h"
 
+#include <memory>
 #include <set>
+#include <string>
 
-#include "absl/strings/match.h"
+#include "absl/status/status.h"
+#include "absl/strings/str_cat.h"
 #include "absl/strings/string_view.h"
 #include "common/analysis/lint_rule_status.h"
 #include "common/analysis/matcher/bound_symbol_manager.h"
 #include "common/analysis/matcher/matcher.h"
-#include "common/strings/naming_utils.h"
+#include "common/text/config_utils.h"
 #include "common/text/symbol.h"
 #include "common/text/syntax_tree_context.h"
 #include "common/text/token_info.h"
+#include "re2/re2.h"
 #include "verilog/CST/module.h"
 #include "verilog/CST/type.h"
 #include "verilog/CST/verilog_matchers.h"
@@ -41,17 +45,27 @@ using verible::LintViolation;
 using verible::SyntaxTreeContext;
 using verible::matcher::Matcher;
 
-static constexpr absl::string_view kMessage =
-    "Interface names must use lower_snake_case naming convention "
-    "and end with _if.";
+static constexpr absl::string_view kLowerSnakeCaseWithSuffixRegex =
+    "[a-z_0-9]+(_if)";
+static constexpr absl::string_view kDefaultStyleRegex =
+    kLowerSnakeCaseWithSuffixRegex;
+
+InterfaceNameStyleRule::InterfaceNameStyleRule()
+    : style_regex_(
+          std::make_unique<re2::RE2>(kDefaultStyleRegex, re2::RE2::Quiet)) {}
 
 const LintRuleDescriptor &InterfaceNameStyleRule::GetDescriptor() {
   static const LintRuleDescriptor d{
       .name = "interface-name-style",
       .topic = "interface-conventions",
       .desc =
-          "Checks that `interface` names use lower_snake_case "
-          "naming convention and end with `_if`.",
+          "Checks that 'interface' names follow a naming convention defined by "
+          "a RE2 regular expression. The default regex pattern expects "
+          "\"lower_snake_case\" with a \"_if\" or \"_e\" suffix. Refer to "
+          "https://github.com/chipsalliance/verible/tree/master/verilog/tools/"
+          "lint#readme for more detail on regex patterns.",
+      .param = {{"style_regex", std::string(kDefaultStyleRegex),
+                 "A regex used to check interface name style."}},
   };
   return d;
 }
@@ -61,6 +75,10 @@ static const Matcher &InterfaceMatcher() {
   return matcher;
 }
 
+std::string InterfaceNameStyleRule::CreateViolationMessage() {
+  return absl::StrCat("Interface name does not match the naming convention ",
+                      "defined by regex pattern: ", style_regex_->pattern());
+}
 void InterfaceNameStyleRule::HandleSymbol(const verible::Symbol &symbol,
                                           const SyntaxTreeContext &context) {
   verible::matcher::BoundSymbolManager manager;
@@ -70,13 +88,21 @@ void InterfaceNameStyleRule::HandleSymbol(const verible::Symbol &symbol,
     identifier_token = GetInterfaceNameToken(symbol);
     name = identifier_token->text();
 
-    if (!verible::IsLowerSnakeCaseWithDigits(name) ||
-        !absl::EndsWith(name, "_if")) {
-      violations_.insert(LintViolation(*identifier_token, kMessage, context));
+    if (!RE2::FullMatch(name, *style_regex_)) {
+      violations_.insert(
+          LintViolation(*identifier_token, CreateViolationMessage(), context));
     }
   }
 }
 
+absl::Status InterfaceNameStyleRule::Configure(
+    absl::string_view configuration) {
+  using verible::config::SetRegex;
+  absl::Status s = verible::ParseNameValues(
+      configuration, {{"style_regex", SetRegex(&style_regex_)}});
+  return s;
+}
+
 LintRuleStatus InterfaceNameStyleRule::Report() const {
   return LintRuleStatus(violations_, GetDescriptor());
 }

verilog/analysis/checkers/interface_name_style_rule.h

@@ -15,32 +15,46 @@
 #ifndef VERIBLE_VERILOG_ANALYSIS_CHECKERS_INTERFACE_NAME_STYLE_RULE_H_
 #define VERIBLE_VERILOG_ANALYSIS_CHECKERS_INTERFACE_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/syntax_tree_lint_rule.h"
 #include "common/text/symbol.h"
 #include "common/text/syntax_tree_context.h"
+#include "re2/re2.h"
 #include "verilog/analysis/descriptions.h"
 
 namespace verilog {
 namespace analysis {
 
-// InterfaceNameStyleRule checks that all interface names use
-// lower_snake_case naming convention and end with "_if".
+// InterfaceNameStyleRule checks that all interface names follow
+// a naming convention matching a regex pattern.
 class InterfaceNameStyleRule : public verible::SyntaxTreeLintRule {
  public:
   using rule_type = verible::SyntaxTreeLintRule;
 
-  static const LintRuleDescriptor& GetDescriptor();
+  InterfaceNameStyleRule();
 
-  void HandleSymbol(const verible::Symbol& symbol,
-                    const verible::SyntaxTreeContext& context) final;
+  static const LintRuleDescriptor &GetDescriptor();
+
+  std::string CreateViolationMessage();
+
+  void HandleSymbol(const verible::Symbol &symbol,
+                    const verible::SyntaxTreeContext &context) final;
 
   verible::LintRuleStatus Report() const final;
 
+  absl::Status Configure(absl::string_view configuration) final;
+
  private:
   std::set<verible::LintViolation> violations_;
+
+  // A regex to check the style against
+  std::unique_ptr<re2::RE2> style_regex_;
 };
 
 }  // namespace analysis

verilog/analysis/checkers/interface_name_style_rule_test.cc

@@ -27,6 +27,7 @@ namespace analysis {
 namespace {
 
 using verible::LintTestCase;
+using verible::RunConfiguredLintTestCases;
 using verible::RunLintTestCases;
 
 TEST(InterfaceNameStyleRuleTest, ValidInterfaceDeclarationNames) {
@@ -90,6 +91,30 @@ TEST(InterfaceNameStyleRuleTest, InvalidInterfaceDeclarationNames) {
   RunLintTestCases<VerilogAnalyzer, InterfaceNameStyleRule>(kTestCases);
 }
 
+TEST(InterfaceNameStyleRuleTest, UpperSnakeCaseTests) {
+  constexpr int kToken = SymbolIdentifier;
+  const std::initializer_list<LintTestCase> kTestCases = {
+      {""},
+      {"interface FOO_IF; endinterface"},
+      {"interface GOOD_NAME_IF; endinterface"},
+      {"interface B_A_Z_IF; endinterface"},
+      {"interface ", {kToken, "HelloWorld"}, "; endinterface"},
+      {"interface ", {kToken, "_baz"}, "; endinterface"},
+      {"interface ", {kToken, "Bad_name"}, "; endinterface"},
+      {"interface ", {kToken, "bad_Name"}, "; endinterface"},
+      {"interface ", {kToken, "Bad2"}, "; endinterface"},
+      {"interface ", {kToken, "very_Bad_name"}, "; endinterface"},
+      {"interface ", {kToken, "wrong_ending"}, "; endinterface"},
+      {"interface ", {kToken, "_if"}, "; endinterface"},
+      {"interface ", {kToken, "_i_f"}, "; endinterface"},
+      {"interface ", {kToken, "i_f"}, "; endinterface"},
+      {"interface ", {kToken, "_"}, "; endinterface"},
+      {"interface ", {kToken, "foo_"}, "; endinterface"},
+  };
+  RunConfiguredLintTestCases<VerilogAnalyzer, InterfaceNameStyleRule>(
+      kTestCases, "style_regex:[A-Z_0-9]+(_IF)");
+}
+
 }  // namespace
 }  // namespace analysis
 }  // namespace verilog