Skip to content

Commit

Permalink
Adding regex configuration for macro names
Browse files Browse the repository at this point in the history
  • Loading branch information
sconwayaus committed Jul 9, 2024
1 parent 1d393d4 commit 0ab6dd3
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 22 deletions.
4 changes: 3 additions & 1 deletion verilog/analysis/checkers/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
77 changes: 60 additions & 17 deletions verilog/analysis/checkers/macro_name_style_rule.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -31,25 +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)),
violation_message_(absl::StrCat(
"Macro name does not match the naming convention ",
"defined by regex pattern: ", style_regex_->pattern())) {}

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. Exceptions are made for UVM like macros, "
"where macros named 'uvm_*' and 'UVM_*' follow 'lower_snake_case' "
"and 'UPPER_SNAKE_CASE' naming conventions respectively.\n"
"Example common regex patterns:\n"
" lower_snake_case: \"[a-z_0-9]+\"\n"
" UPPER_SNAKE_CASE: \"[A-Z_0-9]+\"\n"
" Title_Snake_Case: \"[A-Z]+[a-z0-9]*(_[A-Z0-9]+[a-z0-9]*)*\"\n"
" Sentence_snake_case: \"([A-Z0-9]+[a-z0-9]*_?)([a-z0-9]*_*)*\"\n"
" camelCase: \"([a-z0-9]+[A-Z0-9]*)+\"\n"
" PascalCaseRegexPattern: \"([A-Z0-9]+[a-z0-9]*)+\"\n"
"RE2 regular expression syntax documentation can be found at "
"https://github.com/google/re2/wiki/syntax\n",
.param = {{"style_regex", std::string(upper_snake_case_regex),
"A regex used to check macro names style."}},
};
return d;
}
Expand Down Expand Up @@ -84,19 +115,18 @@ 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, violation_message_));
}
}
state_ = State::kNormal;
Expand All @@ -109,6 +139,19 @@ 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_)}});
if (!s.ok()) return s;

violation_message_ =
absl::StrCat("Macro name does not match the naming convention ",
"defined by regex pattern: ", style_regex_->pattern());

return absl::OkStatus();
}

LintRuleStatus MacroNameStyleRule::Report() const {
return LintRuleStatus(violations_, GetDescriptor());
}
Expand Down
23 changes: 19 additions & 4 deletions verilog/analysis/checkers/macro_name_style_rule.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,38 @@
#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;

static const LintRuleDescriptor &GetDescriptor();
MacroNameStyleRule();

MacroNameStyleRule() = default;
static const LintRuleDescriptor &GetDescriptor();

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 {
Expand All @@ -50,6 +58,13 @@ 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_;

std::string violation_message_;
};

} // namespace analysis
Expand Down
14 changes: 14 additions & 0 deletions verilog/analysis/checkers/macro_name_style_rule_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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

0 comments on commit 0ab6dd3

Please sign in to comment.