From 9c30f5f32ec9b21cae8acad975874451a4fd8b1c Mon Sep 17 00:00:00 2001 From: Steven Conway Date: Sun, 23 Jun 2024 00:44:26 +1000 Subject: [PATCH] Adding regex configuration for macro names --- verilog/analysis/checkers/BUILD | 4 +- .../checkers/macro_name_style_rule.cc | 68 ++++++++++++++----- .../analysis/checkers/macro_name_style_rule.h | 21 +++++- .../checkers/macro_name_style_rule_test.cc | 14 ++++ 4 files changed, 86 insertions(+), 21 deletions(-) diff --git a/verilog/analysis/checkers/BUILD b/verilog/analysis/checkers/BUILD index b90f2fe1e..6ba8a69eb 100644 --- a/verilog/analysis/checkers/BUILD +++ b/verilog/analysis/checkers/BUILD @@ -1566,15 +1566,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, ) diff --git a/verilog/analysis/checkers/macro_name_style_rule.cc b/verilog/analysis/checkers/macro_name_style_rule.cc index b1671a068..e9c44a222 100644 --- a/verilog/analysis/checkers/macro_name_style_rule.cc +++ b/verilog/analysis/checkers/macro_name_style_rule.cc @@ -14,14 +14,18 @@ #include "verilog/analysis/checkers/macro_name_style_rule.h" +#include #include +#include +#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(upper_snake_case_regex, re2::RE2::Quiet)), + style_lower_snake_case_regex_( + std::make_unique(lower_snake_case_regex, re2::RE2::Quiet)), + style_upper_snake_case_regex_(std::make_unique( + 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(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()); } diff --git a/verilog/analysis/checkers/macro_name_style_rule.h b/verilog/analysis/checkers/macro_name_style_rule.h index 969d4ad24..9d12e93b5 100644 --- a/verilog/analysis/checkers/macro_name_style_rule.h +++ b/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 #include +#include +#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 violations_; + + // A regex to check the style against + std::unique_ptr style_regex_; + std::unique_ptr style_lower_snake_case_regex_; + std::unique_ptr style_upper_snake_case_regex_; }; } // namespace analysis diff --git a/verilog/analysis/checkers/macro_name_style_rule_test.cc b/verilog/analysis/checkers/macro_name_style_rule_test.cc index 1a643a25b..0b95f45b8 100644 --- a/verilog/analysis/checkers/macro_name_style_rule_test.cc +++ b/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(kTestCases); } +TEST(MacroNameStyleRuleTest, LowerSnakeCaseTests) { + const std::initializer_list 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( + kTestCases, "style_regex:[a-z_0-9]+"); +} + } // namespace } // namespace analysis } // namespace verilog