Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding regex configuration for macro names #2212

Merged
merged 1 commit into from
Aug 5, 2024
Merged

Adding regex configuration for macro names #2212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion verilog/analysis/checkers/BUILD
View file
Open in desktop
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
68 changes: 51 additions & 17 deletions verilog/analysis/checkers/macro_name_style_rule.cc
View file
Open in desktop
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,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<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)) {}

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<verilog_tokentype>(token.token_enum());
const absl::string_view text(token.text());
Expand Down Expand Up @@ -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;
Expand All @@ -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());
}
Expand Down
21 changes: 18 additions & 3 deletions verilog/analysis/checkers/macro_name_style_rule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <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;

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 {
Expand All @@ -50,6 +60,11 @@ 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_;
};

} // namespace analysis
Expand Down
14 changes: 14 additions & 0 deletions verilog/analysis/checkers/macro_name_style_rule_test.cc
View file
Open in desktop
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
Loading