From 43592ad9c874e9b75cdcaddb18225ecb38dfead8 Mon Sep 17 00:00:00 2001 From: Steven Conway Date: Fri, 21 Jun 2024 22:35:09 +1000 Subject: [PATCH 1/3] Added regex configuration to the interface name rule --- verilog/analysis/checkers/BUILD | 4 +- .../checkers/interface_name_style_rule.cc | 53 +++++++++++++------ .../checkers/interface_name_style_rule.h | 24 +++++++-- .../interface_name_style_rule_test.cc | 25 +++++++++ 4 files changed, 85 insertions(+), 21 deletions(-) diff --git a/verilog/analysis/checkers/BUILD b/verilog/analysis/checkers/BUILD index b90f2fe1e..e2dd054c4 100644 --- a/verilog/analysis/checkers/BUILD +++ b/verilog/analysis/checkers/BUILD @@ -2017,7 +2017,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", @@ -2026,8 +2026,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 e0b53f28d..87fdfd668 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,42 +45,61 @@ 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 style_default_regex = "[a-z_0-9]+(_if)"; -const LintRuleDescriptor& InterfaceNameStyleRule::GetDescriptor() { +InterfaceNameStyleRule::InterfaceNameStyleRule() + : style_regex_( + std::make_unique(style_default_regex, 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(style_default_regex), + "A regex used to check interface name style."}}, }; return d; } -static const Matcher& InterfaceMatcher() { +static const Matcher &InterfaceMatcher() { static const Matcher matcher(NodekInterfaceDeclaration()); return matcher; } -void InterfaceNameStyleRule::HandleSymbol(const verible::Symbol& symbol, - const SyntaxTreeContext& context) { +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; absl::string_view name; - const verible::TokenInfo* identifier_token; + const verible::TokenInfo *identifier_token; if (InterfaceMatcher().Matches(symbol, &manager)) { 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 From babc4f8ad044e3f133a327dcca1683712cc0f89a Mon Sep 17 00:00:00 2001 From: Steven Conway Date: Fri, 21 Jun 2024 22:35:09 +1000 Subject: [PATCH 2/3] Added regex configuration to the interface name rule --- verilog/analysis/checkers/BUILD | 4 +- .../checkers/interface_name_style_rule.cc | 43 ++++++++++++++----- .../checkers/interface_name_style_rule.h | 24 ++++++++--- .../interface_name_style_rule_test.cc | 25 +++++++++++ 4 files changed, 80 insertions(+), 16 deletions(-) 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..87fdfd668 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,24 @@ 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 style_default_regex = "[a-z_0-9]+(_if)"; + +InterfaceNameStyleRule::InterfaceNameStyleRule() + : style_regex_( + std::make_unique(style_default_regex, 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(style_default_regex), + "A regex used to check interface name style."}}, }; return d; } @@ -61,6 +72,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 +85,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 From 82f489c36b7f4368e3f3ab237044dfd22cf957ba Mon Sep 17 00:00:00 2001 From: Steven Conway Date: Tue, 6 Aug 2024 22:26:32 +1000 Subject: [PATCH 3/3] Fixing constant name style --- verilog/analysis/checkers/interface_name_style_rule.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/verilog/analysis/checkers/interface_name_style_rule.cc b/verilog/analysis/checkers/interface_name_style_rule.cc index 87fdfd668..397a828a5 100644 --- a/verilog/analysis/checkers/interface_name_style_rule.cc +++ b/verilog/analysis/checkers/interface_name_style_rule.cc @@ -45,11 +45,14 @@ using verible::LintViolation; using verible::SyntaxTreeContext; using verible::matcher::Matcher; -static constexpr absl::string_view style_default_regex = "[a-z_0-9]+(_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(style_default_regex, re2::RE2::Quiet)) {} + std::make_unique(kDefaultStyleRegex, re2::RE2::Quiet)) {} const LintRuleDescriptor &InterfaceNameStyleRule::GetDescriptor() { static const LintRuleDescriptor d{ @@ -61,7 +64,7 @@ const LintRuleDescriptor &InterfaceNameStyleRule::GetDescriptor() { "\"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(style_default_regex), + .param = {{"style_regex", std::string(kDefaultStyleRegex), "A regex used to check interface name style."}}, }; return d;