diff --git a/verilog/analysis/checkers/BUILD b/verilog/analysis/checkers/BUILD index 83f3d3723..f723853cd 100644 --- a/verilog/analysis/checkers/BUILD +++ b/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, ) diff --git a/verilog/analysis/checkers/interface_name_style_rule.cc b/verilog/analysis/checkers/interface_name_style_rule.cc index 985063225..397a828a5 100644 --- a/verilog/analysis/checkers/interface_name_style_rule.cc +++ b/verilog/analysis/checkers/interface_name_style_rule.cc @@ -14,17 +14,21 @@ #include "verilog/analysis/checkers/interface_name_style_rule.h" +#include #include +#include -#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(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()); } diff --git a/verilog/analysis/checkers/interface_name_style_rule.h b/verilog/analysis/checkers/interface_name_style_rule.h index 5a82a5ace..bd500e2c4 100644 --- a/verilog/analysis/checkers/interface_name_style_rule.h +++ b/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 #include +#include +#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 violations_; + + // A regex to check the style against + std::unique_ptr style_regex_; }; } // namespace analysis diff --git a/verilog/analysis/checkers/interface_name_style_rule_test.cc b/verilog/analysis/checkers/interface_name_style_rule_test.cc index 2a9478128..3ef0edc1b 100644 --- a/verilog/analysis/checkers/interface_name_style_rule_test.cc +++ b/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(kTestCases); } +TEST(InterfaceNameStyleRuleTest, UpperSnakeCaseTests) { + constexpr int kToken = SymbolIdentifier; + const std::initializer_list 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( + kTestCases, "style_regex:[A-Z_0-9]+(_IF)"); +} + } // namespace } // namespace analysis } // namespace verilog